Page MenuHomePhabricator
Create Task
Maniphest T4782

Provide short revision status summary in Differential query results?
Closed, WontfixPublic
Actions

Description

I'd like to propose a small feature (and I'd be interested in taking a stab at coding it).

revision-summary.png (277×943 px, 60 KB)

Overview:

It would be great to show the stats of a revision in the revision query results, like in the mockup. When hovering over the summary, it could show a tooltip with a slightly more detailed version of the summary.

Reason:

My team has a policy of waiting for two reviewers to approve a revision before the author can land it. When looking at the list of open revisions, it is not apparent which have been "Accepted" by a single reviewer, and which have been approved by more than one; we have to open each revision individually to see what's *truly* still pending.

If we were able to quickly scan some stats, (# Accepts, # Rejections, # Comments), I believe it would make the experience better.

Bonus:

If on mouse-over, a more detailed summary could appear, including user name, review status, # of comments, etc. etc. -- that could be even cooler :)

Please let me know if you think this would be a welcome addition to the project. If so, maybe I could get some pointers to help me find out where to start with the code?

Thanks.

Related Objects
Search...

Event Timeline

• mholden created this task.Apr 10 2014, 6:40 AM
• mholden assigned this task to epriestley.
• mholden raised the priority of this task from to Needs Triage.
• mholden updated the task description. (Show Details)
• mholden added a project: Differential.
• mholden added a subscriber: • mholden.
epriestley added a comment.Apr 10 2014, 3:19 PM

I think the most straightforward approach here is for us to to T418, and then you can add this field locally without much effort, and maybe we can eventually adopt something similar in the upstream.

T418 isn't very complicated technically, but I think it will take some work and iterations to get right, since if we do the simple/dumb approach it will look like a pile of butts and not be flexible enough to implement this kind of feature.

T731 discusses alternate acceptance criteria (all reviewers, at least N reviewers, etc.)

epriestley edited this Maniphest Task.Apr 10 2014, 3:19 PM
epriestley added a comment.Apr 10 2014, 3:20 PM

(Particularly, the relevant part of T418 is allowing custom fields to appear in these list interfaces. Once they can do that it's easy to add a small custom field to implement this behavior.)

cburroughs added a subscriber: cburroughs.Aug 26 2014, 3:05 PM
chad removed epriestley as the assignee of this task.Nov 1 2014, 8:14 PM
chad triaged this task as Wishlist priority.
chad added a project: Custom Fields.
epriestley closed this task as Wontfix.Aug 23 2016, 7:27 PM
epriestley claimed this task.
epriestley added a subscriber: epriestley.

I built a similar UI as part of D15926, but we declined to actually bring it upstream. It's possible that we may revisit this in the future, or that it may become possible via T418, but I don't currently plan to pursue this.

For the actual root issue (2 reviewers must accept), see T731 and T10574. There have also been bucketing changes (see discussion of this, among other issues, T10939). You can also selectively make reviewers blocking at arc diff time with Reviewers: reviewer_one!, reviewer_two!, which may be a partial workaround in some cases.

Herald added a subscriber: eadler. · View Herald TranscriptAug 23 2016, 7:27 PM
Phabricator · Built by Phacility