Page MenuHomePhabricator
Create Task
Maniphest T4144

Users disagree on what "Blocking Others", "Action Required" and "Waiting on Others" mean, and/or which groups should exist
Closed, ResolvedPublic
Actions

Description

The Differential dashboard shows three categories "Blocking Others", "Action Required", and "Waiting on Others". I have suggestions for improvements. I'm sure different people have different takes on this, so please let me know if a mailing list discussion is a better venue for this than a Maniphest task.

The current problems (for me):

  • The English phrase "Action Required" seems like a superset of the phrase "Blocking Others", but the actual list isn't a superset of the other. I feel like even though I now use this dashboard once or twice a day, this oddity still makes my brain stutter for a moment.
  • The "Action Required" list combines "Need to fix" and "Need to close". My state of mind is different when I'm deciding which of those review types I feel like tackling at the moment. It would be nice if they were separate lists. Maybe I'll eventually train my brain to look at the green stripe, but at this point I guess I feel like a separate list would be better.
  • Though the term "Blocking Others" is a nice phrase because it conveys the importance of attending to those reviews, I think "Need to review" makes it more obvious what the group is. (Though maybe I'm wrong about what the "Blocking Others" list includes?)
  • The "Waiting on Others" list combines "Waiting on someone to review my revision" and "Waiting on someone to push a revision I reviewed". I usually care a lot more about finding the first category than the second. Again, even though there's a green stripe, I think it would be nicer if these were separate categories. I even be fine if the second category wasn't shown on the dashboard.

What I think I want is the following four categories:

  • Need to review
  • Need to make changes
  • Need to close
  • Waiting on review

And then maybe the following two additional categories, though I wouldn't mind if these weren't shown on the dashboard:

  • Someone else needs to make changes (I'm either reviewer or CC)
  • Someone else needs to close (I'm either reviewer or CC)

(NOTE: I didn't take the time to figure out precisely what each category covered, so I apologize if I messed that up.)

Revisions and Commits

rP Phabricator
D15925rPd46378df208f Modernize "Responsible Users" tokenizer and add "exact(user)" token
D15924rP42d49be47bfc Change Differential revision buckets to focus on "next required action"
D15921rP3a727c31e2f9 Modernize DifferentialRevisionSearchEngine

Related Objects

Event Timeline

cakoose created this task.Nov 21 2013, 10:25 PM
cakoose raised the priority of this task from to Needs Triage.
cakoose updated the task description. (Show Details)
cakoose added a project: Differential.
cakoose added a subscriber: cakoose.
epriestley added subscribers: jimbru, brent, chad, epriestley.Apr 24 2014, 12:47 PM

◀ Merged tasks: T4055.

epriestley added a comment.Apr 24 2014, 12:47 PM

T4055 possibly contemplates a "Need to close, but not yet, because someone else needs to close a dependency first".

epriestley triaged this task as Wishlist priority.May 6 2014, 1:29 PM

We haven't seen many other requests in this vein, and the ones we have seen (T4055, T2511, this) want different things.

In the short term, T3583 (essentially, letting you build your own view as a dashboard) is probably the best attack we have on this.

joshuaspence added a subscriber: joshuaspence.May 29 2015, 11:55 PM
chad mentioned this in T9263: Provide ability to query for all revisions currently blocked by a specific project reviewer.Aug 26 2015, 1:31 AM
epriestley mentioned this in T9372: Plan the future of "Bucketing" Query UIs.Sep 9 2015, 6:34 PM
chad merged a task: T10417: Active Differentials are wrongly indicated as blocking when "request changes" is used.Feb 22 2016, 8:06 PM
chad added subscribers: scode, eadler.
chad merged a task: T8080: Differential shown as "Blocking Others".Feb 22 2016, 8:16 PM
chad added subscribers: michaeljs1990, Luke081515.2, jhurwitz and 2 others.
epriestley renamed this task from Differential dashboard: Making the list of reviews more useful. to Users disagree on what "Blocking Others", "Action Required" and "Waiting on Others" mean, and/or which groups should exist.Feb 22 2016, 8:34 PM
eadler added a project: Restricted Project.Feb 22 2016, 10:10 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.
chad merged a task: T10487: Reviews can return to "blocking others" even after approval .Mar 1 2016, 8:38 PM
chad added a subscriber: ajsecord.
epriestley merged a task: T10515: If I have accepted a revision it still shows up as "blocking others".Mar 4 2016, 4:24 PM
epriestley added a subscriber: • randallli.
epriestley mentioned this in T10574: A pathway toward "All Reviewers Must Accept".Mar 12 2016, 4:50 PM
chad mentioned this in T10689: Display slightly more information about status of revision from main page..Mar 29 2016, 3:22 PM
epriestley mentioned this in T10939: Support for OWNERS files.May 11 2016, 12:57 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.May 13 2016, 6:06 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.May 13 2016, 9:15 PM
epriestley added a revision: D15921: Modernize DifferentialRevisionSearchEngine.May 15 2016, 5:37 PM
epriestley added a revision: D15924: Change Differential revision buckets to focus on "next required action".May 16 2016, 3:17 PM
epriestley added a revision: D15925: Modernize "Responsible Users" tokenizer and add "exact(user)" token.May 16 2016, 3:59 PM
epriestley added a commit: rP3a727c31e2f9: Modernize DifferentialRevisionSearchEngine.May 16 2016, 5:44 PM
epriestley added a commit: rP42d49be47bfc: Change Differential revision buckets to focus on "next required action".
epriestley added a commit: rPd46378df208f: Modernize "Responsible Users" tokenizer and add "exact(user)" token.May 16 2016, 5:46 PM
epriestley closed this task as Resolved.May 16 2016, 7:21 PM
epriestley claimed this task.

There are now six buckets which are more action oriented (Must Review, Ready to Review, Ready to Land, Ready to Update, Waiting on Review, Waiting on Authors) and bucketing is more flexible.

You can use Responsible Users: exact(yourusername) to create a "Personal" dashboard which excludes revisions you are related to only transitively via a package or project. Likewise, you can use Responsible Users: SomeProject to create a project dashboard.

Accepted revisions with open dependencies now show an "Open Dependencies" marker, but don't drop into a different "Almost Ready to Land" bucket.

Reviewer status is now examined individually (as a function of reviewer relationships) instead of in aggregate (as a function of revision status). In particular, accepting a revision will always kick it out of your "Review" queues.

I am declining to implement "Recent Revisions" (T2511) because "recent" is subjective and we haven't seen other requests for this. I think this request predates ApplicationSearch. You can likely build this query yourself today.

None of this is exactly what anyone wanted. If you don't like it, you can now subclass DifferentialRevisionResultBucket and implement your own bucketing algorithm as an extension.

See T10939 for additional context and discussion.

chad mentioned this in T12072: Tighten "Needs Review" category.Jan 6 2017, 4:17 AM
• GoogleLegacy added a subscriber: • GoogleLegacy.Sep 10 2018, 8:35 AM
Phabricator · Built by Phacility