829389 - bzexport should strip reviewers from *first line* of multiline commit messages

Status: RESOLVED FIXED

(Developer Services :: Mercurial: bzexport, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In Mercurial (as with other major version control systems), the semantics of the first line of the commit message are different from later lines.  The first line is the "short" part of the commit mesage, and the rest is the more detailed description.

reviewers go on the first line of the message, not in the detailed description, as in commits like this: https://hg.mozilla.org/mozilla-central/rev/830111e10951

bzexport's code for multiline commit messages and its code for removing reviews are currently backwards.  (I've had this fixed locally for ages; I just never bothered to post the patch anywhere.)

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Guessing sfink as an appropriate reviewer.

I'm actually not sure the behavior without this change is as bad as it was when I wrote it (against 83f819df84d7 or earlier), but I still think it's the right thing to do.

Comment 2

[Steve Fink [:sfink] [:s:]]

Comment on attachment 700790 [details] [diff] [review]
patch

Review of attachment 700790 [details] [diff] [review]:
-----------------------------------------------------------------

::: __init__.py
@@ +719,5 @@
>          desc = desc.lstrip()
>          if desc[0] in ['-', ':', '.']:
>              desc = desc[1:].lstrip()
>  
> +        # Next, just take the first line in case. If there is more than one

Bleh, I've kind of made a mess here.

bzexport explicitly only looks for bug numbers in the first line, by splitting the first line from the rest and only matching against the former. But when I did that, I left in the logic to split it apart again, later.

So really, the code should be cleaned up to split once, at the beginning, and look for both bugs and reviewers in parts[0].

Your fix is totally valid, so land it if you'd like since it's an improvement.

The unintelligible comment ("take the first line in case.") should also be fixed, but that can come with the cleanup too.

It should help that I've somewhat learned Python since writing all this code... :-)

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Each time I think of landing this, I realize there's some change I need to rebase it against and feel like I ought to test it for a little bit before inflicting it on the rest of the world.  This time I'll at least note a reminder...

Comment 4

[(not currently active) Ted Mielczarek]

I'm generally pretty lax about bzexport changes. If it fixes an issue for you then feel free to push it.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/users/tmielczarek_mozilla.com/bzexport/rev/979d18af5993

Comment 6

[Ed Morley [:emorley]]

Ah amazing, this has been bugging me for a while! :-)

Comment 7

[Jonathan Griffin (:jgriffin)]

Attachment: Strip reviewers from *first line* of multiline commit messages.

Comment 8

[Jonathan Griffin (:jgriffin)]

Comment on attachment 799817 [details] [diff] [review]
Strip reviewers from *first line* of multiline commit messages.

Whoops, testing out a new install of bzexport and uploaded this by accident.