Closed Bug 1224141 Opened 9 years ago Closed 9 years ago

MozReview resets reviewer back to original reviewer even when new reviewer is in commit message

Categories

(MozReview Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: dbaron, Assigned: glob)

Details

After bug 686281 comment 175 I redirected a review request from me to :mstange.

Since the re-uploading of the patch in bug 686281 comment 182 changed the review requestee back to me, I asked :cjku in bug 686281 comment 187 to change the r? in the patch's commit message (and redirected the review again right after).  Yet in bug 686281 comment 195 (and the flag changes right before), :cjku uploaded a new patch with "r?:mstange", and it *again* redirected the review request back from mstange to me.

I think it should clearly not have done this final redirection.  The current review requestee in bugzilla was mstange, and the patch also said the requestee should be mstange.
The ':' in the reviewer name breaks our reviewer parsing, so that parsing r?:mstange returns an empty list for reviewers. We recently changed our parser in Bug 1221999, so previously using r?: would have worked.

I think what has happened is that you were flagged with r?:dbaron on the old parser, but when the commit was changed to r?:mstange, the new parser was deployed, and no reviewers were identified from the commit summary.

The current behaviour when no reviewers are identified in the commit summary is to leave the set of reviewers alone so that we don't remove any manually specified reviewers.

If you were a reviewer of the commit in MozReview prior to the push, it would have flagged you to be a reviewer again. I'm assuming this was the case, because we don't support review delegation from within MozReview yet.
Assignee: nobody → glob
looking at the commit history, the r?:name syntax is very unusual - only 28 out of some 189,989 message i looked at use r?:.

when the commits are pushed for review, the following warning is displayed to the patch author:
> (review requests lack reviewers; visit review url to assign reviewers)

unfortunately these reviews were in flight when the change happened; sorry about the confusion here but we shouldn't support the r?: syntax.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.