Thanks to open the bug. Yes, I will use your recommendation below when I check in the patch later.
Thank you Yao Jiewen From: edk2-devel [mailto:[email protected]] On Behalf Of Jordan Justen Sent: Thursday, September 8, 2016 1:06 PM To: Yao, Jiewen <[email protected]>; [email protected] Cc: Chan, Amy <[email protected]> Subject: Re: [edk2] [PATCH] Maintainers.txt: Add Giri as 2nd maintainer On 2016-09-07 15:31:12, Yao, Jiewen wrote: > The problem I am seeing is “inconsistence”. > I opened this bug: https://tianocore.acgmultimedia.com/show_bug.cgi?id=113 For now, can you consider the one I recommended? It is 70 characters: Maintainers.txt: Add Giri as IntelFsp2*Pkg, IntelSiliconPkg maintainer -Jordan > > The > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format > mention 70 char as max. > > But the tool check 76 char. > > > > Can we make then consistent? > > > > Either change wiki to 76 char, or change tool to 70 char. > > > > Thank you > > Yao Jiewen > > > > > > From: Justen, Jordan L > Sent: Thursday, September 8, 2016 1:39 AM > To: Yao, Jiewen <[email protected]<mailto:[email protected]>>; Yao, > Jiewen > <[email protected]<mailto:[email protected]>>; > [email protected]<mailto:[email protected]> > Cc: Chan, Amy <[email protected]<mailto:[email protected]>>; Yao, Jiewen > <[email protected]<mailto:[email protected]>> > Subject: RE: [edk2] [PATCH] Maintainers.txt: Add Giri as 2nd maintainer > > > > On 2016-09-07 01:00:46, Yao, Jiewen wrote: > > Jordan > > > > I have a quick check: > > > > Here is my observation: > > > > 1) From > > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format, > > it mentions: > > > > o The length of 'Pkg-Module: Brief-single-line-summary' should not > > exceed 70 characters > > > > 2) In the PatchCheck.py, we have below code: > > > > if count >= 1 and len(lines[0]) > 76: > > > > self.error('First line of commit message (subject line) ' > + > > > > 'is too long.') > > > > I think the general idea of the subject line is described well here as > the "summary phrase": > > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=bca47613#n493 > > Regarding subject line length, I think that it should work well with > git log --oneline to produce output less than 80 characters. Based on > the standard prefix in git log --oneline, this would indicate that we > should try to use less than 72 characters. I guess the kernel uses > 70-75 characters for guidance. > > > > > 3) You recommendation > > “Maintainers.txt: Add Giri as IntelFsp2*Pkg, IntelSiliconPkg > maintainer”, > > it is 71 char. > > > > However, if we add “[edk2] [PATCH]”, it becomes 85 char. > > > > Hmm, I don't think "[edk2] [PATCH]" should be factored into the > length. The length after applying in git is what should matter. Is > this is a bug in PatchCheck.py? > > I think maybe the subject line length could be a warning from > PatchCheck.py, but it is almost always possible to reword the subject > line fairly easily. If it is difficult to make a good short subject > line, then it might be a sign that the patch should be split. For > example, you could split this patch in 2. One for IntelFsp2*Pkg, and > another for IntelSiliconPkg. (After all, they are 2 separate package > types, so it is reasonable to change them separately.) > > -Jordan > > > > > I am confused on the rule. So I have 2 suggestion: > > > > 1) wiki mentions *70* char, but tool checks *76* char. It seems > > mismatch. We had better fix that. Can we make them consistent? > > > > 2) In most cases, the patch subject is start with *[edk2] > [PATCH]*. > > These 14 additional char had better be excluded in the total 70 or 76 > char > > (I am not sure what is right number), when the subject is calculated. > > > > I have no problem to fix this patch manually. > > > > I do hope we can have consistent rule and tool to help us do the > check. > > > > Thank you > > > > Yao Jiewen > > > > From: edk2-devel [mailto:[email protected]] On Behalf Of > > Yao, Jiewen > > Sent: Wednesday, September 7, 2016 3:21 PM > > To: Justen, Jordan L > <[email protected]<mailto:[email protected]>>; > [email protected]<mailto:[email protected]> > > Cc: Chan, Amy <[email protected]<mailto:[email protected]>> > > Subject: Re: [edk2] [PATCH] Maintainers.txt: Add Giri as 2nd > maintainer > > > > > > > > Jordan > > I got error from BaseTools\Scripts\PatchCheck.py, if I add it. > > > > Can you fix the tool to remove such limitation? > > > > ======================= > > Checking patch file: > C:\home\EdkIITransition\AllComboGit\edk2\0001-Maintainers.t > > xt-Add-Giri-as-2nd-maintainer-to-IntelF.patch > > The commit message format is not valid: > > * First line of commit message (subject line) is too long. > > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format > > The code passed all checks. > > ======================= > > > > From: edk2-devel [mailto:[email protected]] On Behalf > Of Jordan Justen > > Sent: Wednesday, September 7, 2016 3:09 PM > > To: Yao, Jiewen <[email protected]<mailto:[email protected]>>; > [email protected]<mailto:[email protected]> > > Cc: Chan, Amy <[email protected]<mailto:[email protected]>> > > Subject: Re: [edk2] [PATCH] Maintainers.txt: Add Giri as 2nd > maintainer > > > > What about this patch subject instead? > > > > Maintainers.txt: Add Giri as IntelFsp2*Pkg, IntelSiliconPkg maintainer > > > > This way we can see the affected packages from the subject line. > > > > -Jordan > > > > On 2016-09-06 18:21:10, Jiewen Yao wrote: > > > Add Giri as 2nd maintainer to IntelFsp2*Pkg and IntelSiliconPkg. > > > > > > Cc: Giri P Mudusuru > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > Cc: Amy Chan > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Jiewen Yao > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > --- > > > Maintainers.txt | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/Maintainers.txt b/Maintainers.txt > > > index b2e679d..6ac7085 100644 > > > --- a/Maintainers.txt > > > +++ b/Maintainers.txt > > > @@ -130,10 +130,12 @@ M: Jeff Fan > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > IntelFsp2Pkg > > > W: > https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > > > M: Jiewen Yao > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > +M: Giri P Mudusuru > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > > > > IntelFsp2WrapperPkg > > > W: > https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > > > M: Jiewen Yao > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > +M: Giri P Mudusuru > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > > > > IntelFspPkg > > > W: > https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > > > @@ -146,6 +148,7 @@ M: Jiewen Yao > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > IntelSiliconPkg > > > W: > https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > > > M: Jiewen Yao > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > +M: Giri P Mudusuru > <[email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>>> > > > > > > MdeModulePkg > > > W: > https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg > > > -- > > > 2.7.4.windows.1 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > > [email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>> > > > https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > > edk2-devel mailing list > > > [email protected]<mailto:[email protected]<mailto:[email protected]%3cmailto:[email protected]>> > > https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > > edk2-devel mailing list > > [email protected]<mailto:[email protected]> > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected]<mailto:[email protected]> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
