public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
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

      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