1227923 - prevent review flags from being set on mozreview attachments

Status: RESOLVED WONTFIX

(bugzilla.mozilla.org Graveyard :: Extensions: MozReview Integration, defect)

Description

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

If a reviewer flags a patch r+ from bugzilla, the r+ does not get forwarded to reviewboard. Therefore the reviewer will receive a review request again when a new patch set is uploaded to reviewboard.

For example:
A patch is r+ in bug 1172796 comment 43. However, when the patch author uploads a new patch set, he needs to manually carry r+ in bugzilla in bug 1172796 comment 60. 

We need a way to carry r+ in reviewboard, or have better integration between bugzilla and reviewboard.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

(FWIW, I still review mozreview patches by downloading them and looking at in an editor. That way I get similar view to the changes as what Bugzilla's "Details" gives - and that happens to easiest way for me to understand the changes.)

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

If the review requests come from mozreview, you might want to click the "Finish Review ..." and check "Ship it" checkbox to grant r+. This could save you from being spammed by review request mails until we get the better integration between mozreview and bugzilla.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

well, often I want to add some comments too.
I should try MozReview again, but so far it hasn't helped with code reviews I do.
But it is getting better all the time, so I guess at some point it will be fine.

Comment 4

[Ralph Giles (:rillian)]

This issue has also been bothering me. It's not just that obsolete review requests clutter the dashboard, but if my reviewer marks approval in bugzilla instead of mozreview, I cannot use the autolander.

Comment 5

[Gregory Szorc [:gps]]

I see 2 options here:

1) Disallow r+ to be granted from Bugzilla for MozReview attachments (require interactions to occur in MozReview)
2) Having r+ initiated in Bugzilla propagate to a review in MozReview

Bugzilla already has access to a valid Bugzilla API key so it can authenticate with MozReview using that, so no special account/permissions for #2 should be needed.

glob: can you please weigh in on this old bug?

Comment 6

[:glob ✱]

my preference, by a long way, is to prevent review flag manipulation in bugzilla.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Since giving r- is these days possible in mozreview, there are fewer reasons to r+/- in bugzilla.

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

But it is still not possible to set r+/r- back to r? nowadays. (I know updating patch would reset r- to r?, but sometimes people may want to reset it directly without updating patches, e.g. re-request review with a detailed explanation.)

Comment 9

[Mark Côté [:mcote]]

Indeed, bug 1195661 will have to be fixed before we do this.

Comment 11

[Mark Côté [:mcote]]

Also see bug 1258057, which sort of solves this (the attachment details page would still exist, and we may or may not want to disable review-flag setting there).

Comment 12

[Sylvestre Ledru [:Sylvestre]]

We are not using mozreview anymore, closing all remaining bugs.