Re: [PATCH] Maintainers.txt: Add Giri as 2nd maintainer

[Jordan Justen <jordan.l.justen@intel.com>]

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