1119430 - Bizarre behavior with multiple reviewboard reviews cross-pollinating

Status: RESOLVED DUPLICATE of bug 1116901

(MozReview Graveyard :: General, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I had reviewboard reviews requested from me in bug 869208 (<https://reviewboard.mozilla.org/r/2053/>) and bug 1119302 (<https://reviewboard.mozilla.org/r/2213/>).

I did the latter review first, and had a review comment.  For some reason, instead of the comment being placed in bug 1119302 it was placed in bug 869208 comment 13.

Furthermore, if I look at the diff in https://reviewboard.mozilla.org/r/2053/ it seems to include the changes from bug 1119302.  Not sure whether that matters here.

Comment 1

[Gregory Szorc [:gps]]

I reckon r2059 has a parent review associated with bug 869208 and thus all review request activity is sent there.

The current behavior involving commit series referencing multiple bugs is crap. It unfairly conforms developers to a single-bug-per-branch workflow. The review tool should not be limiting workflows, especially real-life ones where multiple commits/bugs are "stacked" on top of each other.

This will be addressed by the end of Q1, hopefully by the end of January.

Comment 2

[Benjamin Smedberg]

FWIW, I didn't intend to request review from bz in bug 869208. That was already complete and had been reviewed by aklotz.

Sequence of events:
* Created the patch for bug 869208 and uploaded it, r?aklotz, all seemed to work ok. `hg push review -r.`
* On top of that first patch, I wrote a patch for bug 1119302. But I mistyped the bug number in that commit, 1119291 instead of 1119302. I pushed to review using `hg push review -r.::.`
* I added review?bz in https://reviewboard.mozilla.org/r/2213/ and published it.
* I noticed that no comment or review appeared in either bug 1119302, saw the incorrect bug#, amended the patch, and pushed it again using `hg push review -r.::.`
* Still no automatic review request showed up, so I manually added the review request in bug 1119302.

Comment 4

[Mark Côté [:mcote]]

Ah, yes, the main problem was caused from having two different bugs in the two commits of one parent review request.  In this case, only the first bug (869208 in this case) is updated when the parent request is updated.  Looks like bsmedberg pushed the review for bug 1119302 on top of the review for bug 869208.  You would have needed to have two different hg bookmarks here, not have 1119302 on top of 869208, to generate two completely separate review requests.

Only "ship its" from the parent request result in r+s, btw (this is relevant to bug 1119416, which I duped here).  "Ship it" in a child request will cause a comment to be added to the bug, but that's it.  Also, it is expected behaviour that the diff of the parent request (2053) includes the diffs of both children (2213, 2055).

So this was a combination of user error (pushing a new review request on top of an old, unrelated one), but also MozReview allowing users to use two different bugs for two commits in the same parent request but then not indicating that only the first bug would be used.  All of this is essentially caused by our not-very-great UI for distinguishing parent and child requests, which we're going to fix, as gps noted.

Comment 5

[Benjamin Smedberg]

Will pushing entirely separate reviews from the same stack of patches be supported in the future? Requiring a separate bookmark for each patch would be seriously annoying. I thought that the base::tip syntax documented at http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/review-requests.html was intended for specifically this purpose so that I could push a review from a stacked set of unrelated changes.

Comment 6

[Mark Côté [:mcote]]

Yeah I think so.  I believe bug 1116901 best summarizes that problem; duping this to it.  Bug 1054673, bug 1055021, and bug 1115584 are all also exhibited here too, I think, to lesser degrees.