* [PATCH] Maintainers.txt: Add Giri as 2nd maintainer @ 2016-09-07 1:21 Jiewen Yao 2016-09-07 3:50 ` Mudusuru, Giri P 2016-09-07 7:08 ` Jordan Justen 0 siblings, 2 replies; 12+ messages in thread From: Jiewen Yao @ 2016-09-07 1:21 UTC (permalink / raw) To: edk2-devel; +Cc: Giri P Mudusuru, Amy Chan Add Giri as 2nd maintainer to IntelFsp2*Pkg and IntelSiliconPkg. Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com> Cc: Amy Chan <amy.chan@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiewen Yao <jiewen.yao@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.fan@intel.com> IntelFsp2Pkg W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg M: Jiewen Yao <jiewen.yao@intel.com> +M: Giri P Mudusuru <giri.p.mudusuru@intel.com> IntelFsp2WrapperPkg W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg M: Jiewen Yao <jiewen.yao@intel.com> +M: Giri P Mudusuru <giri.p.mudusuru@intel.com> IntelFspPkg W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com> IntelSiliconPkg W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg M: Jiewen Yao <jiewen.yao@intel.com> +M: Giri P Mudusuru <giri.p.mudusuru@intel.com> MdeModulePkg W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg -- 2.7.4.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 1:21 [PATCH] Maintainers.txt: Add Giri as 2nd maintainer Jiewen Yao @ 2016-09-07 3:50 ` Mudusuru, Giri P 2016-09-07 7:08 ` Jordan Justen 1 sibling, 0 replies; 12+ messages in thread From: Mudusuru, Giri P @ 2016-09-07 3:50 UTC (permalink / raw) To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Chan, Amy Thanks Jiewen. Reviewed-by: Giri P Mudusuru <giri.p.mudusuru@intel.com> > -----Original Message----- > From: Yao, Jiewen > Sent: Tuesday, September 6, 2016 6:21 PM > To: edk2-devel@lists.01.org > Cc: Mudusuru, Giri P <giri.p.mudusuru@intel.com>; Chan, Amy > <amy.chan@intel.com> > Subject: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer > > Add Giri as 2nd maintainer to IntelFsp2*Pkg and IntelSiliconPkg. > > Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com> > Cc: Amy Chan <amy.chan@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao <jiewen.yao@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.fan@intel.com> > IntelFsp2Pkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > M: Jiewen Yao <jiewen.yao@intel.com> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com> > > IntelFsp2WrapperPkg > W: > https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > M: Jiewen Yao <jiewen.yao@intel.com> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com> > > IntelFspPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com> > IntelSiliconPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > M: Jiewen Yao <jiewen.yao@intel.com> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com> > > MdeModulePkg > W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg > -- > 2.7.4.windows.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 1:21 [PATCH] Maintainers.txt: Add Giri as 2nd maintainer Jiewen Yao 2016-09-07 3:50 ` Mudusuru, Giri P @ 2016-09-07 7:08 ` Jordan Justen 2016-09-07 7:21 ` Yao, Jiewen 1 sibling, 1 reply; 12+ messages in thread From: Jordan Justen @ 2016-09-07 7:08 UTC (permalink / raw) To: Jiewen Yao, edk2-devel; +Cc: Amy Chan 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.mudusuru@intel.com> > Cc: Amy Chan <amy.chan@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao <jiewen.yao@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.fan@intel.com> > IntelFsp2Pkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > M: Jiewen Yao <jiewen.yao@intel.com> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com> > > IntelFsp2WrapperPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > M: Jiewen Yao <jiewen.yao@intel.com> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com> > > IntelFspPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com> > IntelSiliconPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > M: Jiewen Yao <jiewen.yao@intel.com> > +M: Giri P Mudusuru <giri.p.mudusuru@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 > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 7:08 ` Jordan Justen @ 2016-09-07 7:21 ` Yao, Jiewen 2016-09-07 8:00 ` Yao, Jiewen 0 siblings, 1 reply; 12+ messages in thread From: Yao, Jiewen @ 2016-09-07 7:21 UTC (permalink / raw) To: Justen, Jordan L, edk2-devel@lists.01.org; +Cc: Chan, Amy 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-bounces@lists.01.org] On Behalf Of Jordan Justen Sent: Wednesday, September 7, 2016 3:09 PM To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org Cc: Chan, Amy <amy.chan@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.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > Cc: Amy Chan <amy.chan@intel.com<mailto:amy.chan@intel.com>> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@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.fan@intel.com<mailto:jeff.fan@intel.com>> > IntelFsp2Pkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > > IntelFsp2WrapperPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > > IntelFspPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > IntelSiliconPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 7:21 ` Yao, Jiewen @ 2016-09-07 8:00 ` Yao, Jiewen 2016-09-07 17:39 ` Jordan Justen 0 siblings, 1 reply; 12+ messages in thread From: Yao, Jiewen @ 2016-09-07 8:00 UTC (permalink / raw) To: Yao, Jiewen, Justen, Jordan L, edk2-devel@lists.01.org Cc: Chan, Amy, Yao, Jiewen 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: * 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.') 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. 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-bounces@lists.01.org] On Behalf Of Yao, Jiewen Sent: Wednesday, September 7, 2016 3:21 PM To: Justen, Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org Cc: Chan, Amy <amy.chan@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-bounces@lists.01.org] On Behalf Of Jordan Justen Sent: Wednesday, September 7, 2016 3:09 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Chan, Amy <amy.chan@intel.com<mailto:amy.chan@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.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > Cc: Amy Chan <amy.chan@intel.com<mailto:amy.chan@intel.com<mailto:amy.chan@intel.com%3cmailto:amy.chan@intel.com>>> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@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.fan@intel.com<mailto:jeff.fan@intel.com<mailto:jeff.fan@intel.com%3cmailto:jeff.fan@intel.com>>> > IntelFsp2Pkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > IntelFsp2WrapperPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > IntelFspPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > IntelSiliconPkg > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 8:00 ` Yao, Jiewen @ 2016-09-07 17:39 ` Jordan Justen 2016-09-07 17:49 ` Ard Biesheuvel 2016-09-07 22:31 ` Yao, Jiewen 0 siblings, 2 replies; 12+ messages in thread From: Jordan Justen @ 2016-09-07 17:39 UTC (permalink / raw) To: Yao, Jiewen, Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Chan, Amy, Yao, Jiewen 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-bounces@lists.01.org] On Behalf Of > Yao, Jiewen > Sent: Wednesday, September 7, 2016 3:21 PM > To: Justen, Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org > Cc: Chan, Amy <amy.chan@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-bounces@lists.01.org] On Behalf Of Jordan Justen > Sent: Wednesday, September 7, 2016 3:09 PM > To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org > Cc: Chan, Amy <amy.chan@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.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > > Cc: Amy Chan <amy.chan@intel.com<mailto:amy.chan@intel.com>> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@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.fan@intel.com<mailto:jeff.fan@intel.com>> > > IntelFsp2Pkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > > > > IntelFsp2WrapperPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > > > > IntelFspPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > > @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > > IntelSiliconPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 17:39 ` Jordan Justen @ 2016-09-07 17:49 ` Ard Biesheuvel 2016-09-07 18:08 ` Jordan Justen 2016-09-07 22:31 ` Yao, Jiewen 1 sibling, 1 reply; 12+ messages in thread From: Ard Biesheuvel @ 2016-09-07 17:49 UTC (permalink / raw) To: Jordan Justen; +Cc: Yao, Jiewen, edk2-devel@lists.01.org, Chan, Amy On 7 September 2016 at 18:39, Jordan Justen <jordan.l.justen@intel.com> wrote: > 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.) > Please no. It is outright ridiculous to split a maintainer update patch that adds the same person to two packages into two patches, only because the subject line becomes too long otherwise. We are not changing code here, things are not becoming easier to understand by doing so, and 'making the tool happy' is the worst reason I can think of to change a perfectly good patch. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 17:49 ` Ard Biesheuvel @ 2016-09-07 18:08 ` Jordan Justen 0 siblings, 0 replies; 12+ messages in thread From: Jordan Justen @ 2016-09-07 18:08 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: Yao, Jiewen, edk2-devel@lists.01.org, Chan, Amy On 2016-09-07 10:49:21, Ard Biesheuvel wrote: > On 7 September 2016 at 18:39, Jordan Justen <jordan.l.justen@intel.com> wrote: > > > > 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.) > > > > Please no. It is outright ridiculous to split a maintainer update > patch that adds the same person to two packages into two patches, only > because the subject line becomes too long otherwise. We are not > changing code here, things are not becoming easier to understand by > doing so, and 'making the tool happy' is the worst reason I can think > of to change a perfectly good patch. We're not trying to make a tool happy. We're trying to use the tool to help write better patches. Also, it is not like splitting it is difficult. If 5 packages are being updated, I want it to be clear in the subject line which packages are being updated. And, yet, I don't want the subject line length to grow far too long just because many packages are being changed. It does seem like in this case, my proposed subject line captures all the pacakges in one commit and is short enough. Personally, I don't really care to see the person's name in the subject line for package maintainer changes. Based on the subject line, if I'm interested in the package, then I can look at the patch to see who is being added/deleted. -Jordan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 17:39 ` Jordan Justen 2016-09-07 17:49 ` Ard Biesheuvel @ 2016-09-07 22:31 ` Yao, Jiewen 2016-09-08 4:19 ` Mudusuru, Giri P 2016-09-08 5:06 ` Jordan Justen 1 sibling, 2 replies; 12+ messages in thread From: Yao, Jiewen @ 2016-09-07 22:31 UTC (permalink / raw) To: Justen, Jordan L, edk2-devel@lists.01.org; +Cc: Chan, Amy 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.yao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org Cc: Chan, Amy <amy.chan@intel.com>; Yao, Jiewen <jiewen.yao@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-bounces@lists.01.org] On Behalf Of > Yao, Jiewen > Sent: Wednesday, September 7, 2016 3:21 PM > To: Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Chan, Amy <amy.chan@intel.com<mailto:amy.chan@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-bounces@lists.01.org] On Behalf Of Jordan Justen > Sent: Wednesday, September 7, 2016 3:09 PM > To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Chan, Amy <amy.chan@intel.com<mailto:amy.chan@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.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > Cc: Amy Chan <amy.chan@intel.com<mailto:amy.chan@intel.com<mailto:amy.chan@intel.com%3cmailto:amy.chan@intel.com>>> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@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.fan@intel.com<mailto:jeff.fan@intel.com<mailto:jeff.fan@intel.com%3cmailto:jeff.fan@intel.com>>> > > IntelFsp2Pkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > > > IntelFsp2WrapperPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > > > IntelFspPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > > @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > > IntelSiliconPkg > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 22:31 ` Yao, Jiewen @ 2016-09-08 4:19 ` Mudusuru, Giri P 2016-09-08 5:06 ` Jordan Justen 1 sibling, 0 replies; 12+ messages in thread From: Mudusuru, Giri P @ 2016-09-08 4:19 UTC (permalink / raw) To: Yao, Jiewen, Justen, Jordan L, edk2-devel@lists.01.org; +Cc: Chan, Amy 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-bounces@lists.01.org] On Behalf Of Yao, > Jiewen > Sent: Wednesday, September 7, 2016 3:31 PM > To: Justen, Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org > Cc: Chan, Amy <amy.chan@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.yao@intel.com>; Yao, Jiewen > <jiewen.yao@intel.com>; edk2-devel@lists.01.org > Cc: Chan, Amy <amy.chan@intel.com>; Yao, Jiewen <jiewen.yao@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-bounces@lists.01.org] On Behalf Of > > Yao, Jiewen > > Sent: Wednesday, September 7, 2016 3:21 PM > > To: Justen, Jordan L > <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; edk2- > devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Chan, Amy <amy.chan@intel.com<mailto:amy.chan@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-bounces@lists.01.org] On Behalf Of > Jordan Justen > > Sent: Wednesday, September 7, 2016 3:09 PM > > To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Chan, Amy <amy.chan@intel.com<mailto:amy.chan@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.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.m > udusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > > Cc: Amy Chan > <amy.chan@intel.com<mailto:amy.chan@intel.com<mailto:amy.chan@intel.co > m%3cmailto:amy.chan@intel.com>>> > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Jiewen Yao > <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen.yao@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.fan@intel.com<mailto:jeff.fan@intel.com<mailto:jeff.fan@intel.com%3c > mailto:jeff.fan@intel.com>>> > > > IntelFsp2Pkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > > > M: Jiewen Yao > <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen.yao@intel.com>>> > > > +M: Giri P Mudusuru > <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.m > udusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > > > > > IntelFsp2WrapperPkg > > > W: > https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > > > M: Jiewen Yao > <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen.yao@intel.com>>> > > > +M: Giri P Mudusuru > <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.m > udusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > > > > > IntelFspPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > > > @@ -146,6 +148,7 @@ M: Jiewen Yao > <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen.yao@intel.com>>> > > > IntelSiliconPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > > > M: Jiewen Yao > <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@inte > l.com%3cmailto:jiewen.yao@intel.com>>> > > > +M: Giri P Mudusuru > <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.m > udusuru@intel.com%3cmailto:giri.p.mudusuru@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-07 22:31 ` Yao, Jiewen 2016-09-08 4:19 ` Mudusuru, Giri P @ 2016-09-08 5:06 ` Jordan Justen 2016-09-08 5:15 ` Yao, Jiewen 1 sibling, 1 reply; 12+ messages in thread From: Jordan Justen @ 2016-09-08 5:06 UTC (permalink / raw) To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Chan, Amy 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 <jiewen.yao@intel.com>; Yao, Jiewen > <jiewen.yao@intel.com>; edk2-devel@lists.01.org > Cc: Chan, Amy <amy.chan@intel.com>; Yao, Jiewen <jiewen.yao@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-bounces@lists.01.org] On Behalf Of > > Yao, Jiewen > > Sent: Wednesday, September 7, 2016 3:21 PM > > To: Justen, Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org > > Cc: Chan, Amy <amy.chan@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-bounces@lists.01.org] On Behalf Of Jordan Justen > > Sent: Wednesday, September 7, 2016 3:09 PM > > To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org > > Cc: Chan, Amy <amy.chan@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.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > > > Cc: Amy Chan <amy.chan@intel.com<mailto:amy.chan@intel.com>> > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@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.fan@intel.com<mailto:jeff.fan@intel.com>> > > > IntelFsp2Pkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > > > > > > IntelFsp2WrapperPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com>> > > > > > > IntelFspPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > > > @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > > > IntelSiliconPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> > > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer 2016-09-08 5:06 ` Jordan Justen @ 2016-09-08 5:15 ` Yao, Jiewen 0 siblings, 0 replies; 12+ messages in thread From: Yao, Jiewen @ 2016-09-08 5:15 UTC (permalink / raw) To: Justen, Jordan L, edk2-devel@lists.01.org; +Cc: Chan, Amy, Yao, Jiewen 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:edk2-devel-bounces@lists.01.org] On Behalf Of Jordan Justen Sent: Thursday, September 8, 2016 1:06 PM To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org Cc: Chan, Amy <amy.chan@intel.com> 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 <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Yao, Jiewen > <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Chan, Amy <amy.chan@intel.com<mailto:amy.chan@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@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-bounces@lists.01.org] On Behalf Of > > Yao, Jiewen > > Sent: Wednesday, September 7, 2016 3:21 PM > > To: Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Chan, Amy <amy.chan@intel.com<mailto:amy.chan@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-bounces@lists.01.org] On Behalf Of Jordan Justen > > Sent: Wednesday, September 7, 2016 3:09 PM > > To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Chan, Amy <amy.chan@intel.com<mailto:amy.chan@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.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > > Cc: Amy Chan <amy.chan@intel.com<mailto:amy.chan@intel.com<mailto:amy.chan@intel.com%3cmailto:amy.chan@intel.com>>> > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@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.fan@intel.com<mailto:jeff.fan@intel.com<mailto:jeff.fan@intel.com%3cmailto:jeff.fan@intel.com>>> > > > IntelFsp2Pkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg > > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > > > > > IntelFsp2WrapperPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2WrapperPkg > > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@intel.com>>> > > > > > > IntelFspPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelFspPkg > > > @@ -146,6 +148,7 @@ M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > > > IntelSiliconPkg > > > W: https://github.com/tianocore/tianocore.github.io/wiki/IntelSiliconPkg > > > M: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> > > > +M: Giri P Mudusuru <giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com<mailto:giri.p.mudusuru@intel.com%3cmailto:giri.p.mudusuru@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<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-09-08 5:15 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-07 1:21 [PATCH] Maintainers.txt: Add Giri as 2nd maintainer Jiewen Yao 2016-09-07 3:50 ` Mudusuru, Giri P 2016-09-07 7:08 ` Jordan Justen 2016-09-07 7:21 ` Yao, Jiewen 2016-09-07 8:00 ` Yao, Jiewen 2016-09-07 17:39 ` Jordan Justen 2016-09-07 17:49 ` Ard Biesheuvel 2016-09-07 18:08 ` Jordan Justen 2016-09-07 22:31 ` Yao, Jiewen 2016-09-08 4:19 ` Mudusuru, Giri P 2016-09-08 5:06 ` Jordan Justen 2016-09-08 5:15 ` Yao, Jiewen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox