Page MenuHomePhabricator

When accepting revisions, allow users to accept on behalf of a subset of reviewers
ClosedPublic

Authored by epriestley on Mar 22 2017, 4:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 11:54 PM
Unknown Object (File)
Fri, Nov 22, 6:30 PM
Unknown Object (File)
Mon, Nov 18, 12:16 AM
Unknown Object (File)
Thu, Nov 14, 10:45 AM
Unknown Object (File)
Sat, Nov 9, 11:41 PM
Unknown Object (File)
Sat, Nov 9, 10:10 PM
Unknown Object (File)
Sat, Nov 9, 8:56 PM
Unknown Object (File)
Tue, Nov 5, 9:54 PM
Subscribers
None

Details

Summary

Ref T12271. Currenty, when you "Accept" a revision, you always accept it for all reviewers you have authority over.

There are some situations where communication can be more clear if users can accept as only themselves, or for only some packages, etc. T12271 discusses some of these use cases in more depth.

Instead of making "Accept" a blanket action, default it to doing what it does now but let the user uncheck reviewers.

In cases where project/package reviewers aren't in use, this doesn't change anything.

For now, "reject" still acts the old way (reject everything). We could make that use checkboxes too, but I'm not sure there's as much of a use case for it, and I generally want users who are blocking stuff to have more direct accountability in a product sense.

Test Plan
  • Accepted normally.
  • Accepted a subset.
  • Tried to accept none.
  • Tried to accept bogus reviewers.
  • Accepted with myself not a reviewer
  • Accepted with only one reviewer (just got normal "this will be accepted" text).

Screen Shot 2017-03-22 at 9.30.55 AM.png (504×1 px, 60 KB)

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This technically hit the bug fixed in D17534, but seems to have mostly worked prior to that.

This revision is now accepted and ready to land.Mar 22 2017, 8:28 PM
This revision was automatically updated to reflect the committed changes.