On 05/15/19 09:02, Zhang, Shenglei wrote: > Hi Laszlo: > >> -----Original Message----- >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >> Laszlo Ersek >> Sent: Tuesday, May 14, 2019 9:49 PM >> To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zh...@intel.com> >> Cc: Justen, Jordan L <jordan.l.jus...@intel.com>; Ard Biesheuvel >> <ard.biesheu...@linaro.org>; Anthony Perard <anthony.per...@citrix.com>; >> Julien Grall <julien.gr...@linaro.org> >> Subject: Re: [edk2-devel] [PATCH 1/4] OvmfPkg: Update DSC/FDF to use >> NetworkPkg's include fragment file.
>> (4) Therefore, please *prepend* a patch to this series that eliminates >> the [LibraryClasses.common.DXE_DRIVER] resolutions altogether, at first. > > Thanks for your comments! > I'll send a patch first to solve the duplicated library classes about network > in > [LibraryClasses.common.DXE_DRIVER] section. Thanks for that, I'll review it soon. >> (6) Please be consistent with the comments that you add near the >> !include directives. In the current patch, you add comments for the libs >> and the PCDs, but not for the defines or the components. Please stick >> with one style -- either add zero comments, or add comments on all of >> the !includes. > > Actually, for components I add "Network Support" in the comments, which looks > like > aligning with nearby components. Do you thinks it is acceptable? Sorry, I missed the pre-existent comment there, and the fact that you were preserving the comment. So yes, that is fine -- as long as all !include directives uniformly have a comment (newly added, or inherited from preexistent code), it's OK. Please note however that your network *defines* !include still lacks a comment! Please fix that, for consistency with the rest of the !include directives. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40644): https://edk2.groups.io/g/devel/message/40644 Mute This Topic: https://groups.io/mt/31616136/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-