Patch "fix regression issue for platform .map file" was pushed. More comments inline.
Thanks, Bob -----Original Message----- From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek Sent: Friday, November 22, 2019 1:26 AM To: Leif Lindholm <leif.lindh...@linaro.org>; Gao, Liming <liming....@intel.com> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kin...@intel.com>; 'af...@apple.com' <af...@apple.com> Subject: Re: [edk2-devel] Patch List for 201911 stable tag On 11/21/19 11:37, Leif Lindholm wrote: > On Thu, Nov 21, 2019 at 08:57:12AM +0000, Gao, Liming wrote: >> Hi Stewards and all: >> New bugs are for 201911 stable tag. Can you give the comments for them? >> >> Bug List (those all have pass code review): >> https://edk2.groups.io/g/devel/message/50931 [PATCH] MdeModulePkg: >> LzmaCustomDecompressLib.inf don't support EBC anymore > > This can wait until after the stable tag *unless* that causes serious > issues for CI. (And I don't consider non-zero build failures for EBC > to be serious.) The fix only addresses building the component > standalone as part of MdeModulePkg, so it affects no real platforms. I would have beem more permissive with this patch, but I'm OK with the above resolution too. > > I have some comments with regards to the implementation as well, but > I'll give those on the actual patch email. > >> https://edk2.groups.io/g/devel/message/50990 [PATCH V1 1/1] >> MdeModulePkg/Variable: Initialize local variable > > Those two bugs are poster children for why long functions should be > avoided. > > OK for me to include in stable tag. Agreed. > >> https://edk2.groups.io/g/devel/message/50989 [PATCH V2] BaseTools:fix >> regression issue for platform .map file > > OK for me to include *if* the commit message (and bugzilla) are > updated to clarify the issue. "is missing" is passive language, and > hence fails to convey what is causing the problem. Does this mean that > the 'build' command fails to generate these lines? > > Also, a comment on why this change (of moving this hunk 18 lines up) > resolves the issue should be included. Agreed; please add a reminder to the commit message (even on push) about what the specific (missing) lines in the map files are good for in the first place. [Bob] The commit message was updated https://edk2.groups.io/g/devel/message/51212 And the patch was pushed. > >> https://edk2.groups.io/g/devel/message/50936 [PATCH] BaseTools:fixed >> Build failed issue for Non-English OS > > I agree that a fix for this bug must be included in the stable tag. > But both the BZ and the commit message need improving. > And seeing a fix that consists solely of adding "errors='ignore'" > worries me somewhat. > > Please explain *what* goes wrong when using Non-English OS (and which > OS), as well as why "errors='ignore'" does not cause further problems. I think this patch is plain wrong. I'll comment on it in its own thread. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#51214): https://edk2.groups.io/g/devel/message/51214 Mute This Topic: https://groups.io/mt/60556595/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
