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>, "Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer
Date: Thu, 8 Sep 2016 05:15:04 +0000 [thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C50385FB627@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <147331116681.20751.17243067839489821727@jljusten-ivb>
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
prev parent reply other threads:[~2016-09-08 5:15 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
2016-09-08 4:19 ` Mudusuru, Giri P
2016-09-08 5:06 ` Jordan Justen
2016-09-08 5:15 ` Yao, Jiewen [this message]
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=74D8A39837DF1E4DA445A8C0B3885C50385FB627@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