1319738 - commit messages with "r?" but no reviewer are not rejected

Status: RESOLVED FIXED

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

Description

[Carsten Book [:Tomcat]]

In https://reviewboard.mozilla.org/r/93068/#review95192 a r? commit was pushed to the autoland repo https://hg.mozilla.org/integration/autoland/rev/77bcbb91d542fd53db6f5d3683af5442cf279971

i think

a) mozreview should have added the reviewer
and maybe more important
b) mozreview should not allow commits with r? 
there is even a mozhook that should prevent this https://hg.mozilla.org/hgcustom/version-control-tools/file/tip/hghooks/mozhghooks/commit-message.py

    if INVALID_REVIEW_FLAG_RE.search(firstline):
        message("Rev {rev} contains 'r?' in the commit message. Please use 'r=' instead.")
        return False

but seems this does not work here

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Background:

I pushed a patch to reviewboard and at this time I didn't want anyone to review this patch just yet, so I added just "r?" to the commit message.

At a later point I added a reviewer in reviewboard. The patch got r+ and I landed it via autoland.

Expected:
 The patch should have landed with "r=walkingice" added to the commit message

Actual:
 The patch landed with "r?" in the commit message.

Comment 2

[:glob ✱]

there's two issues here: first is the commit description wasn't updated correct, and the second is there should be checks in place to ensure commits with r? can't be pushed.

i've filed bug 1320251 to track the rewriting failure, leaving this bug to track adding a hook.

Comment 3

[:glob ✱]

(In reply to Carsten Book [:Tomcat] from comment #0)
>     if INVALID_REVIEW_FLAG_RE.search(firstline):
>         message("Rev {rev} contains 'r?' in the commit message. Please use
> 'r=' instead.")
>         return False
> 
> but seems this does not work here

the INVALID_REVIEW_FLAG regex is /[\s\.;]r\?\w/, which doesn't match 'r?' due to the trailing \w (see bug 1173881 comment 2).

we'd probably want something like /\br\?(\w|$)/ which matches on 'r?nick' or 'r?' at the end of the line.

Comment 4

[:glob ✱]

Attachment: hghooks: reject commit messages that end in 'r?' (bug 1319738)

Review commit: https://reviewboard.mozilla.org/r/95918/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/95918/

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8814819 [details]
hghooks: reject commit messages that end in 'r?' (bug 1319738)

https://reviewboard.mozilla.org/r/95918/#review96176

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/115809a6a8e1
hghooks: reject commit messages that end in 'r?' r=gps