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: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>; edk2-devel@lists.01.org > Cc: Chan, Amy <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>; edk2-devel@lists.01.org > Cc: Chan, Amy <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>> > > Cc: Amy Chan <amy.c...@intel.com<mailto:amy.c...@intel.com>> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Jiewen Yao > <jiewen....@intel.com<mailto: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>> > > IntelFsp2Pkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > > M: Jiewen Yao <jiewen....@intel.com<mailto:jiewen....@intel.com>> > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto: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>> > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto: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>> > > IntelSiliconPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > > M: Jiewen Yao <jiewen....@intel.com<mailto:jiewen....@intel.com>> > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto: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> > > 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
