Page MenuHomePhabricator

When users accept a revision, allow them to accept for a subset of package/project reviewers they could accept for
Closed, ResolvedPublic

Description

Currently, the "Accept" action shows this:

This currently means "accept on behalf of myself as an individual user, and all projects and packages I have authority over" (the stuff highlighted in yellow in the "Reviewers" list).

There are some edge cases where this isn't quite flexible enough to express intent. For example:

  • You want to unblock a revision on behalf of the project "Spelling Things Correctly Task Force" which you are a member of, but do not want to become the reviewer of record. Your intent is to assert "I have reviewed this change for spelling errors and found none", not "this is good to ship". Right now, if you "Accept" (and there are no other blocking reviewers), the revision will become landable.
  • You own several projects that are reviewers but only want to express an accept/unblock on behalf of some of them (say, "Python Style Police" but not "Serious Security Review" since you feel like you're a little out of your depth).

These cases are normally rare, only associated with advanced Project/Owners workflows, and can generally be worked around with communication or by removing reviewers, but they're relatively reasonable and there's no technical reason we can't let you choose which reviewers you wish to accept on behalf of, like this, and this UI should fit into the product reasonably gracefully:

Accept Revision: [√] As epriestley
                 [√] As "Spelling Things Correctly Elite Task Force"
                 [√] As "Python Style Police"

By default, all the options would be checked. When you would only accept on behalf of yourself, we can omit the options, since accepting on behalf of nothing doesn't make sense. So this UI would only show up when it was relevant and the workflow normally would not change very much, but should provide more flexibility for complicated workflows.

This workflow change should also extend to Audit, which has similar product concerns.

Event Timeline

hskiba added a subscriber: hskiba.Feb 17 2017, 10:52 AM
epriestley closed this task as Resolved.Mar 28 2017, 1:31 PM
epriestley claimed this task.

This has been out in the world for a little bit without exploding. I'd like to maybe do a bit of UI touchup but that can happen after T12272.

T10967 also covers this in general and will probably be open for a bit until we get rid of the double writes and old storage, so followups can go there too.