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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 7 September 2016 at 18:39, Jordan Justen <jordan.l.justen@intel.com> wrote:
> 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.)
>

Please no. It is outright ridiculous to split a maintainer update
patch that adds the same person to two packages into two patches, only
because the subject line becomes too long otherwise. We are not
changing code here, things are not becoming easier to understand by
doing so, and 'making the tool happy' is the worst reason I can think
of to change a perfectly good patch.