public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Mudusuru, Giri P" <giri.p.mudusuru@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
	"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: Thu, 8 Sep 2016 04:19:15 +0000	[thread overview]
Message-ID: <4666AEFED60F8E4198B42BB01DCEABDF76ED48A2@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C50385FAFEF@shsmsx102.ccr.corp.intel.com>

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

  reply	other threads:[~2016-09-08  4:19 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 [this message]
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=4666AEFED60F8E4198B42BB01DCEABDF76ED48A2@ORSMSX113.amr.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