1173881 - Reject commit messages with ' r?'

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Biru [:poiru]]

See: http://git.mozilla.org/?p=integration%2Fgecko-dev.git&a=search&h=HEAD&st=commit&s=%20r%3F

Comment 1

[Biru [:poiru]]

Attachment: Reject commit messages with ' r?'

I didn't find false positives with this simple check (see link in comment 0).

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8621109 [details] [diff] [review]
Reject commit messages with ' r?'

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

::: hghooks/mozhghooks/commit-message.py
@@ +52,5 @@
>      if "[PATCH" in desc:
>          message("Rev {rev} contains git-format-patch \"[PATCH]\" cruft. Use git-format-patch -k to avoid this.")
>          return False
>  
> +    if " r?" in firstline:

This is slightly too greedy for my liking since it will flag "Fix bug with r? detection". We probably want the regexp 'r\?\w' to match "r?" followed by some characters.

Feel free to request review on an updated patch or submit this and file a follow-up bug.

Comment 3

[Biru [:poiru]]

Attachment: Reject commit messages with 'r?'

(In reply to Gregory Szorc [:gps] from comment #2)
> This is slightly too greedy for my liking since it will flag "Fix bug with
> r? detection". We probably want the regexp 'r\?\w' to match "r?" followed by
> some characters.

Done.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8621227 [details] [diff] [review]
Reject commit messages with 'r?'

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

::: hghooks/mozhghooks/commit-message.py
@@ +17,5 @@
>  
>  import re
>  from mercurial.node import hex
>  
> +INVALID_REVIEW_FLAG_RE = re.compile(r'[\s;.]r\?\w')

This may not be greedy enough! But it should be good for most scenarios. When it comes to potentially getting in the way of user workflows, I'd rather cast a narrow net before a wide one.

Comment 5

[Biru [:poiru]]

http://hg.mozilla.org/hgcustom/version-control-tools/rev/f74a7a7f5e86

Comment 6

[Gregory Szorc [:gps]]

Deployed to production.