This has come up a few times in bugs I've mentored. Basically I co-review the bug (I'm not a peer on the module), the other person adds some comments which default to 'issues'. New contributor pushes changes, we both r+ but I can't land because there are open 'issues' left. I can clear my own, but I can't clear the co-reviewer's. I think the assumption is that, as a reviewer, once I r+ something all issues are considered cleared. Some possible solutions: - Allow landing a bug with open issues - Clear issues on r+ - Allow co-reviewers to clear issues - Get rid of issues - Don't make comments default to being an issue I prefer the first option, but would be happy with other improvements.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1288136
I don't see how this is a duplicate of a nine month old "Autoland should warn about if issues marked unsolved, rather than be unavailable in the UI" bug, I have no interest in being warned. I already know why the thing can't land, I just think it's wrong.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
resolving that bug would address the issue you raise here. > Allow landing a bug with open issues this is unlikely to happen; problems raised during review should be address prior to landing > Clear issues on r+ this doesn't mesh well with multiple reviewers - if reviewer A raises an issue that needs to be fixed, reviewer B setting r+ doesn't necessarily mean that those issues do not need to be addressed. this is an extension of an r- overriding an r+. > Allow co-reviewers to clear issues i like this idea, except anyone who can land the code should be able to clear issues, not just co-reviewers. > Get rid of issues this is a non-starter due to the scope of this change > Don't make comments default to being an issue i suspect it's too late in the game to change this default: morphing this bug to the most viable option.
Summary: Allow landing a bug that has open issues → Allow users with the ability to land code to resolve any open issues
(In reply to Byron Jones ‹:glob› from comment #3) > > Clear issues on r+ > > this doesn't mesh well with multiple reviewers - if reviewer A raises an > issue that needs to be fixed, reviewer B setting r+ doesn't necessarily mean > that those issues do not need to be addressed. this is an extension of an > r- overriding an r+. I could see clearing Reviewer A's issues when Reviewer A gives an r+, in this case that would have sufficed. > morphing this bug to the most viable option. Thanks, that seems reasonable.
MozReview is now obsolete. Please use Phabricator instead. Closing this bug.
Status: REOPENED → RESOLVED
Closed: 2 years ago → 9 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.