Allow users with the ability to land code to resolve any open issues

RESOLVED INVALID

Status

enhancement
RESOLVED INVALID
2 years ago
9 months ago

People

(Reporter: erahm, Unassigned)

Tracking

Details

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 ago9 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.