Closed Bug 1169499 Opened 4 years ago Closed 4 years ago
Reviewers reset unless r? syntax is present
MozReview Request: reviewboard: we should not reset reviewers if none are specified in commit summary (bug 1169499); r?gps
39 bytes, text/x-review-board-request
I just pushed a new review series to https://reviewboard.mozilla.org/r/7951/. I had reviewers set on all the old published review requests. After pushing new, rebased commits, reviewers have disappeared from the draft review requests (I'm not using r? syntax to define reviewers via commit messages). I reckon the clearing of reviewers is related to supporting the r? syntax. If reviewers are specified in the review request, I don't think a new push w/o reviewers should clear the existing reviewers.
This is essentially the same problem as Bug 1164132 but I think it is worth solving separately as it is clearly undesirable behaviour that is easy to avoid.
Assignee: nobody → dminor
Priority: -- → P1
reviewboard: we should not reset reviewers if none are specified in commit summary (bug 1169499); r?gps This is a symptom of a larger problem where specifying reviewers in the commit summary causes any reviewers specified in the UI to be lost. Solving that larger problem requires us to have a good means of identifying reviewers which were specified in the UI and which were specified in the commit summary. In the mean time, if no reviewers are specified in the commit summary we can skip setting the reviewers on the draft, which will preserve reviewers that were set in the UI if none are specified in the commit.
Attachment #8612805 - Flags: review?(gps)
Comment on attachment 8612805 [details] MozReview Request: reviewboard: we should not reset reviewers if none are specified in commit summary (bug 1169499); r?gps https://reviewboard.mozilla.org/r/9635/#review8467 ::: pylib/reviewboardmods/reviewboardmods/pushhooks.py:347 (Diff revision 1) > + else: > + draft = rr.get_or_create_draft( > + summary=commit['message'].splitlines(), > + description=commit['message']) I liked your `props` dict approach from above to avoid code duplication. I think you should change this code while you are here. ::: hgext/reviewboard/tests/test-specify-reviewers.t:332 (Diff revision 1) > - $ hg commit --amend -m 'Bug 1 - Amended stuff; r?romulus, r?remus' > + $ hg commit --amend -m 'Bug 1 - Amended more stuff; r?romulus, r?remus' I don't think you needed to change the commit message here. But it doesn't matter too much.
Attachment #8612805 - Flags: review?(gps) → review+
Thanks, pushed to: https://hg.mozilla.org/hgcustom/version-control-tools
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
(In reply to Dan Minor [:dminor] from comment #4) > Thanks, pushed to: https://hg.mozilla.org/hgcustom/version-control-tools Well, obviously :) The revision is: https://hg.mozilla.org/hgcustom/version-control-tools/rev/665dfb80447a
You need to log in before you can comment on or make changes to this bug.