public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length
@ 2020-01-02 12:16 Philippe Mathieu-Daudé
  2020-01-09  5:35 ` [edk2-devel] " Bob Feng
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-02 12:16 UTC (permalink / raw)
  To: devel
  Cc: Philippe Mathieu-Daude, Liming Gao, Jordan Justen, Yonghong Zhu,
	Zhichao Gao

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=113

Strip the trailing characters before checking the subject line is
less than 72 characters.

Fixes: e61406708c83f
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
v2: Use rstrip() (Jordan Justen)
v1: https://edk2.groups.io/g/devel/message/52425
---
 BaseTools/Scripts/PatchCheck.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 2a4e6f603e79..9668025798da 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -196,7 +196,7 @@ class CommitMessageCheck:
             self.error('Empty commit message!')
             return
 
-        if count >= 1 and len(lines[0]) >= 72:
+        if count >= 1 and len(lines[0].rstrip()) >= 72:
             self.error('First line of commit message (subject line) ' +
                        'is too long.')
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [edk2-devel] [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length
  2020-01-02 12:16 [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length Philippe Mathieu-Daudé
@ 2020-01-09  5:35 ` Bob Feng
  2020-01-20  9:11   ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 5+ messages in thread
From: Bob Feng @ 2020-01-09  5:35 UTC (permalink / raw)
  To: devel@edk2.groups.io, philmd@redhat.com
  Cc: Gao, Liming, Justen, Jordan L, Zhu, Yonghong, Gao, Zhichao

Reviewed-by: Bob Feng <bob.c.feng@intel.com>

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Philippe Mathieu-Daudé
Sent: Thursday, January 2, 2020 8:17 PM
To: devel@edk2.groups.io
Cc: Philippe Mathieu-Daude <philmd@redhat.com>; Gao, Liming <liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: [edk2-devel] [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=113

Strip the trailing characters before checking the subject line is less than 72 characters.

Fixes: e61406708c83f
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
v2: Use rstrip() (Jordan Justen)
v1: https://edk2.groups.io/g/devel/message/52425
---
 BaseTools/Scripts/PatchCheck.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py index 2a4e6f603e79..9668025798da 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -196,7 +196,7 @@ class CommitMessageCheck:
             self.error('Empty commit message!')
             return
 
-        if count >= 1 and len(lines[0]) >= 72:
+        if count >= 1 and len(lines[0].rstrip()) >= 72:
             self.error('First line of commit message (subject line) ' +
                        'is too long.')
 
--
2.21.0





^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-devel] [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length
  2020-01-09  5:35 ` [edk2-devel] " Bob Feng
@ 2020-01-20  9:11   ` Philippe Mathieu-Daudé
  2020-01-20 10:27     ` Laszlo Ersek
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20  9:11 UTC (permalink / raw)
  To: Feng, Bob C, devel@edk2.groups.io
  Cc: Gao, Liming, Justen, Jordan L, Zhu, Yonghong, Gao, Zhichao

Hi Liming,

On 1/9/20 6:35 AM, Feng, Bob C wrote:
> Reviewed-by: Bob Feng <bob.c.feng@intel.com>

Is the Review-by tag from Bob enough to get this patch merged?

Thanks,

Phil.

> 
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Philippe Mathieu-Daudé
> Sent: Thursday, January 2, 2020 8:17 PM
> To: devel@edk2.groups.io
> Cc: Philippe Mathieu-Daude <philmd@redhat.com>; Gao, Liming <liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: [edk2-devel] [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=113
> 
> Strip the trailing characters before checking the subject line is less than 72 characters.
> 
> Fixes: e61406708c83f
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
> ---
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> v2: Use rstrip() (Jordan Justen)
> v1: https://edk2.groups.io/g/devel/message/52425
> ---
>   BaseTools/Scripts/PatchCheck.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py index 2a4e6f603e79..9668025798da 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -196,7 +196,7 @@ class CommitMessageCheck:
>               self.error('Empty commit message!')
>               return
>   
> -        if count >= 1 and len(lines[0]) >= 72:
> +        if count >= 1 and len(lines[0].rstrip()) >= 72:
>               self.error('First line of commit message (subject line) ' +
>                          'is too long.')
>   
> --
> 2.21.0
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-devel] [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length
  2020-01-20  9:11   ` Philippe Mathieu-Daudé
@ 2020-01-20 10:27     ` Laszlo Ersek
  2020-01-20 10:51       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2020-01-20 10:27 UTC (permalink / raw)
  To: devel, philmd, Feng, Bob C
  Cc: Gao, Liming, Justen, Jordan L, Zhu, Yonghong, Gao, Zhichao

On 01/20/20 10:11, Philippe Mathieu-Daudé wrote:
> Hi Liming,
> 
> On 1/9/20 6:35 AM, Feng, Bob C wrote:
>> Reviewed-by: Bob Feng <bob.c.feng@intel.com>
> 
> Is the Review-by tag from Bob enough to get this patch merged?

Yes, see the "M" role:

BaseTools
F: BaseTools/
W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
M: Bob Feng <bob.c.feng@intel.com>
M: Liming Gao <liming.gao@intel.com>

... In fact, the patch has been committed as 2649a735b249e.

Unfortunately:

- your name has been messed up in the "Author" meta-datum :(

- we don't directly know who committed the patch, becaus "Committer"
only says "mergify[bot]"

Now, based on the subject and the commit date of the patch, I can find
some related notification emails in my "github.com email notifications"
folder. From those, the revelant (merged) GitHub PR seems to be:

https://github.com/tianocore/edk2/pull/283

I agree it's annoying to not have confirmation on the mailing list,
directly in your patch's thread. The only thing I can recommend is,
check the git history periodically, for recently committed patches,
authored by yourself.

Personally, on the maintainer side, I always make sure to confirm a
merge on-list, separately.

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-devel] [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length
  2020-01-20 10:27     ` Laszlo Ersek
@ 2020-01-20 10:51       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20 10:51 UTC (permalink / raw)
  To: Laszlo Ersek, devel, Feng, Bob C
  Cc: Gao, Liming, Justen, Jordan L, Zhu, Yonghong, Gao, Zhichao

On 1/20/20 11:27 AM, Laszlo Ersek wrote:
> On 01/20/20 10:11, Philippe Mathieu-Daudé wrote:
>> Hi Liming,
>>
>> On 1/9/20 6:35 AM, Feng, Bob C wrote:
>>> Reviewed-by: Bob Feng <bob.c.feng@intel.com>
>>
>> Is the Review-by tag from Bob enough to get this patch merged?
> 
> Yes, see the "M" role:
> 
> BaseTools
> F: BaseTools/
> W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
> M: Bob Feng <bob.c.feng@intel.com>
> M: Liming Gao <liming.gao@intel.com>
> 
> ... In fact, the patch has been committed as 2649a735b249e.
> 
> Unfortunately:
> 
> - your name has been messed up in the "Author" meta-datum :(

Now I see it, OK. I don't understand charset encoding enough to avoid 
mojibake, but I'll try few things with PatchCheck.

> - we don't directly know who committed the patch, becaus "Committer"
> only says "mergify[bot]"
> 
> Now, based on the subject and the commit date of the patch, I can find
> some related notification emails in my "github.com email notifications"
> folder. From those, the revelant (merged) GitHub PR seems to be:
> 
> https://github.com/tianocore/edk2/pull/283

Thanks Bob for pushing it, sorry for the noise.

Thanks Laszlo for checking and updating me.

> I agree it's annoying to not have confirmation on the mailing list,
> directly in your patch's thread. The only thing I can recommend is,
> check the git history periodically, for recently committed patches,
> authored by yourself.
> 
> Personally, on the maintainer side, I always make sure to confirm a
> merge on-list, separately.
> 
> Thanks
> Laszlo
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-01-20 10:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-02 12:16 [PATCH v2] BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length Philippe Mathieu-Daudé
2020-01-09  5:35 ` [edk2-devel] " Bob Feng
2020-01-20  9:11   ` Philippe Mathieu-Daudé
2020-01-20 10:27     ` Laszlo Ersek
2020-01-20 10:51       ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox