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/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<mailto:jordan.l.jus...@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 > > > > 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.mudus...@intel.com%3cmailto:giri.p.mudus...@intel.com>>> > > Cc: Amy Chan > <amy.c...@intel.com<mailto:amy.c...@intel.com<mailto:amy.c...@intel.com%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....@intel.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%3cmailto: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....@intel.com%3cmailto:jiewen....@intel.com>>> > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto:giri.p.mudus...@intel.com<mailto:giri.p.mudus...@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....@intel.com%3cmailto:jiewen....@intel.com>>> > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto:giri.p.mudus...@intel.com<mailto:giri.p.mudus...@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....@intel.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....@intel.com%3cmailto:jiewen....@intel.com>>> > > +M: Giri P Mudusuru > <giri.p.mudus...@intel.com<mailto:giri.p.mudus...@intel.com<mailto:giri.p.mudus...@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-devel@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-devel@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
