Agee with Jiewen to keep the doc and tool consistent. Also the patch prefix "[edk2][PATCH]" should be excluded as it is not part of the commit message title.
Thanks, -Giri > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, > Jiewen > Sent: Wednesday, September 7, 2016 3:31 PM > To: Justen, Jordan L <jordan.l.jus...@intel.com>; edk2-devel@lists.01.org > Cc: Chan, Amy <amy.c...@intel.com> > Subject: Re: [edk2] [PATCH] Maintainers.txt: Add Giri as 2nd maintainer > > The problem I am seeing is “inconsistence”. > > 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 <jiewen....@intel.com>; Yao, Jiewen > <jiewen....@intel.com>; edk2-devel@lists.01.org > Cc: Chan, Amy <amy.c...@intel.com>; Yao, Jiewen <jiewen....@intel.com> > 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/Documentatio > n/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:edk2-devel-boun...@lists.01.org] On Behalf Of > > Yao, Jiewen > > Sent: Wednesday, September 7, 2016 3:21 PM > > To: Justen, Jordan L > <jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>; edk2- > de...@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Chan, Amy <amy.c...@intel.com<mailto:amy.c...@intel.com>> > > 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:edk2-devel-boun...@lists.01.org] On Behalf Of > Jordan Justen > > Sent: Wednesday, September 7, 2016 3:09 PM > > To: Yao, Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Chan, Amy <amy.c...@intel.com<mailto:amy.c...@intel.com>> > > 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 > <giri.p.mudus...@intel.com<mailto:giri.p.mudus...@intel.com<mailto:giri.p.m > udus...@intel.com%3cmailto:giri.p.mudus...@intel.com>>> > > > Cc: Amy Chan > <amy.c...@intel.com<mailto:amy.c...@intel.com<mailto:amy.c...@intel.co > m%3cmailto:amy.c...@intel.com>>> > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Jiewen Yao > <jiewen....@intel.com<mailto:jiewen....@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen....@intel.com>>> > > > --- > > > 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 > <jeff....@intel.com<mailto:jeff....@intel.com<mailto:jeff....@intel.com%3c > mailto:jeff....@intel.com>>> > > > IntelFsp2Pkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > > > M: Jiewen Yao > <jiewen....@intel.com<mailto:jiewen....@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen....@intel.com>>> > > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto:giri.p.mudus...@intel.com<mailto:giri.p.m > udus...@intel.com%3cmailto:giri.p.mudus...@intel.com>>> > > > > > > IntelFsp2WrapperPkg > > > W: > https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > > > M: Jiewen Yao > <jiewen....@intel.com<mailto:jiewen....@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen....@intel.com>>> > > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto:giri.p.mudus...@intel.com<mailto:giri.p.m > udus...@intel.com%3cmailto:giri.p.mudus...@intel.com>>> > > > > > > IntelFspPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > > > @@ -146,6 +148,7 @@ M: Jiewen Yao > <jiewen....@intel.com<mailto:jiewen....@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen....@intel.com>>> > > > IntelSiliconPkg > > > W: > > https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > > > M: Jiewen Yao > <jiewen....@intel.com<mailto:jiewen....@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen....@intel.com>>> > > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto:giri.p.mudus...@intel.com<mailto:giri.p.m > udus...@intel.com%3cmailto:giri.p.mudus...@intel.com>>> > > > > > > MdeModulePkg > > > W: > https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg > > > -- > > > 2.7.4.windows.1 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2- > de...@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > > > https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2- > de...@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > > https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
