From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "Justen, Jordan L" <jordan.l.justen@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Chan, Amy" <amy.chan@intel.com>
Subject: Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer
Date: Wed, 7 Sep 2016 22:31:12 +0000 [thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C50385FAFEF@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <147326996342.12894.6103663724254201876@jljusten-ivb>
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
next prev parent reply other threads:[~2016-09-07 22:31 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2016-09-08 4:19 ` Mudusuru, Giri P
2016-09-08 5:06 ` Jordan Justen
2016-09-08 5:15 ` Yao, Jiewen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74D8A39837DF1E4DA445A8C0B3885C50385FAFEF@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox