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. > >> 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 (#51106): https://edk2.groups.io/g/devel/message/51106 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] -=-=-=-=-=-=-=-=-=-=-=-
