Just some quick remarks after a comparison with v3: On 07/27/20 18:48, Vladimir Olovyannikov via groups.io wrote: > Introduce an http client utilizing EDK2 HTTP protocol, to > allow fast image downloading from http/https servers. > HTTP download speed is usually faster than tftp. > The client is based on the same approach as tftp dynamic command, and > uses the same UEFI Shell command line parameters. This makes it easy > integrating http into existing UEFI Shell scripts. > Note that to enable HTTP download, feature Pcd > gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must > be set to TRUE. > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2860 > > Signed-off-by: Vladimir Olovyannikov <vladimir.olovyanni...@broadcom.com> > CC: Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com> > CC: Laszlo Ersek <ler...@redhat.com>
(1) These "CC" lines are not formatted correctly -- they might do the job as far as git-send-email is concerned, but they don't satisfy "PatchCheck.py": > ShellPkg/DynamicCommand: add HttpDynamicCommand > The commit message format is not valid: > * 'CC' should be 'Cc' > * 'CC' should be 'Cc' > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format The exit status is 255, so this would break the CI run again. Please run "PatchCheck.py" locally before posting, and/or submit a personal CI build. [...] > + if (UserNicName != NULL) { > + if (StrCmp (NicName, UserNicName) != 0) { > + Status = EFI_NOT_FOUND; Change is new since v4, but not documented in the v5 changelog. (The code may be OK, but please help reviewers with the v(n) -> v(n+1) changelogs.) [...] > + if (ShellCommandLineGetFlag (CheckPackage, L"-m")) { > + Context.Flags |= DL_FLAG_TIME; > + } (2) The "-m" flag has not been removed from here [...] > +// Download Flags > +#define DL_FLAG_TIME BIT0 // Show elapsed time. (3) and here > +".SH SYNOPSIS\r\n" > +" \r\n" > +"HTTP [-i interface] [-l port] [-t timeout] [-s size] [-m] [-k]\r\n" (4) and here [...] > +" -m Measure and report download time (in seconds). \r\n" (5) and here. I suggest waiting for ShellPkg owner feedback before posting v6. Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#63363): https://edk2.groups.io/g/devel/message/63363 Mute This Topic: https://groups.io/mt/75826968/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
