Hi Laszlo, Yes. The commit message is very important. I'll care more on the commit message in future code review. For this patch I'll ask submitter to enter a BZ and refine this patch commit message.
Thanks, Bob -----Original Message----- From: Laszlo Ersek [mailto:[email protected]] Sent: Wednesday, April 17, 2019 9:23 PM To: [email protected]; Fan, ZhijuX <[email protected]>; Feng, Bob C <[email protected]> Cc: Gao, Liming <[email protected]> Subject: Re: [edk2-devel] [PATCH] BaseTools:Makefiles cause data errors on some platforms Hi, On 04/17/19 04:02, Fan, ZhijuX wrote: > As the Dict is unordered, an error occurs when using the Dict while > creating a new item.So for now, use OrdereDict instead of Dict. this problem statement is impenetrable. - What is the exact symptom? - What triggers it, and where? - When we make the dict ordered, why does that mitigate the issue? - Do we have a BZ for this? The subject line of the patch is similarly unhelpful: "data errors on some platforms". It's basically devoid of information. --*-- Recently I've also seen a number of commits (with different code changes) where the subject line was reused identically.... Yup: 1 2914e8153dd3 BaseTools: Fix corner-cases of --hash feature 2 1b8caf0d87ea BaseTools: Fix corner-cases of --hash feature 3 f2b5e04acad2 BaseTools: Fix corner-cases of --hash feature 4 db4d47fd3ae9 BaseTools: Fix corner-cases of --hash feature What's really sad here is that the commit messages are otherwise pretty helpful, it's just that the subject lines were chosen in an unfortunate way. Bob: please enforce higher standards for commit messages under BaseTools. People *care*. (I certainly do.) Good commit messages teach details about the subsystem or module. Thanks, Laszlo > > Cc: Bob Feng <[email protected]> > Cc: Liming Gao <[email protected]> > Signed-off-by: Zhiju.Fan <[email protected]> > --- > BaseTools/Source/Python/AutoGen/GenMake.py | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py > b/BaseTools/Source/Python/AutoGen/GenMake.py > index 7562dc68b3..426e6ca51a 100644 > --- a/BaseTools/Source/Python/AutoGen/GenMake.py > +++ b/BaseTools/Source/Python/AutoGen/GenMake.py > @@ -429,7 +429,7 @@ cleanlib: > self.CommonFileDependency = [] > self.FileListMacros = {} > self.ListFileMacros = {} > - self.ObjTargetDict = {} > + self.ObjTargetDict = OrderedDict() > self.FileCache = {} > self.LibraryBuildCommandList = [] > self.LibraryFileList = [] > @@ -954,11 +954,9 @@ cleanlib: > self.ListFileMacros[T.IncListFileMacro] = [] > if self._AutoGenObject.BuildRuleFamily == TAB_COMPILER_MSFT > and Type == TAB_C_CODE_FILE: > NewFile = self.PlaceMacro(str(T), self.Macros) > - if self.ObjTargetDict.get(T.Target.SubDir): > - self.ObjTargetDict[T.Target.SubDir].add(NewFile) > - else: > + if not self.ObjTargetDict.get(T.Target.SubDir): > self.ObjTargetDict[T.Target.SubDir] = set() > - self.ObjTargetDict[T.Target.SubDir].add(NewFile) > + self.ObjTargetDict[T.Target.SubDir].add(NewFile) > > Deps = [] > CCodeDeps = [] > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39282): https://edk2.groups.io/g/devel/message/39282 Mute This Topic: https://groups.io/mt/31207805/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
