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

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Tomcat, Assigned: glob)

Tracking

Production

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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
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.
(Assignee)

Comment 2

2 years ago
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.
(Assignee)

Comment 3

2 years ago
(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.
Assignee: nobody → glob
Component: Autoland → Mercurial: hg.mozilla.org
Product: MozReview → Developer Services
QA Contact: hwine
Summary: Autoland should not allow incomplete commit messages (like missing reviewer) → commit messages with "r?" but no reviewer are not rejected
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8814819 [details]
hghooks: reject commit messages that end in 'r?' (bug 1319738)

https://reviewboard.mozilla.org/r/95918/#review96176
Attachment #8814819 - Flags: review?(gps) → review+

Comment 6

2 years ago
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/115809a6a8e1
hghooks: reject commit messages that end in 'r?' r=gps
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.