On 12/05/19 17:12, Philippe Mathieu-Daude wrote:
> git-format-patch uses the '---' separator to mark the end of the
> commit message. Anything after the separator is stripped by git-am,
> it is pointless to check this part.
>
> Cc: Bob Feng <bob.c.f...@intel.com>
> Cc: Liming Gao <liming....@intel.com>
> Signed-off-by: Philippe Mathieu-Daude <phi...@redhat.com>
> ---
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Cc: Leif Lindholm <leif.lindh...@linaro.org>
> ---
> BaseTools/Scripts/PatchCheck.py | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index 2a4e6f603e79..8bcf857a7d15 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -209,6 +209,10 @@ class CommitMessageCheck:
> 'empty.')
>
> for i in range(2, count):
> + if lines[i].strip() == '---':
> + # '---' marks the end of the commit message.
> + count = i
> + break
> if (len(lines[i]) >= 76 and
> len(lines[i].split()) > 1 and
> not lines[i].startswith('git-svn-id:')):
>
This shortcut seems to prevent two checks (on the affected area):
- line length check,
- separation of the signature block by an empty line.
While preventing the second check in the affected area certainly makes
sense, I think preserving the line length check would be nice.
For example, "git-notes" places notes in that area, and if someone uses
a text editor configuration in which hard line breaks are not inserted,
we could end up with a humongous note there. Even though it would not be
applied by git-am, I think we should not get such notes posted to the list.
(This text editor configuration is not theoretical: earlier we used to
get actual commit messages with no hard line breaks.)
So... I don't want to complicate this for you needlessly, but you
mention in the series blurb that this patch is a bugfix. What is the bug?
Hmmm... does the check (mistakenly) fire if the diffstat is very wide?
(It can easily be very wide, because our pathnames are long.) Hmmm.
Okay, I agree that's a much more practical problem than what I describe
above. So:
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51798): https://edk2.groups.io/g/devel/message/51798
Mute This Topic: https://groups.io/mt/67003936/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
