Page MenuHomePhabricator
Create Task
Maniphest T5230

Accepted revision flag is not erased/adjusted on revision update
Closed, InvalidPublic
Actions

Description

If a reviewer comments on a diff, his/her status in the "Reviewers" section is Commented. When the diff gets updated, the icon and the status changes to Commented on a prior diff.

Similarly, when the reviewer rejects the diff and the diff gets updated, the the icon changes and the status is Rejected prior diff.

However, if he/she accepts the diff, and the diff gets updated, the icons and the status is still the same: Accepted.

That's very weird, because I would assume that changed code needs a fresh review and confirmation. Also, in the "queue" on author's Phab home page the diff is colored green, which can confuse the author and he/she can push the commit without realizing that the latest update hasn't been reviewed yet. I'm not even sure if the revision appears in the queue of reviewers who have already accepted the prior diff, or not.

Our Phab version is 20140518.git3a81f8c.

Event Timeline

kparal created this task.May 30 2014, 12:10 PM
kparal raised the priority of this task from to Needs Triage.
kparal updated the task description. (Show Details)
kparal added a project: Differential.
kparal added subscribers: kparal, tflink.
kparal added a comment.May 30 2014, 12:22 PM

Correction, there seems to be no Commented on a prior diff (it just displays a note next to prior diff's inline comments). But that doesn't affect this issue much.

chad added a subscriber: chad.May 30 2014, 2:12 PM

Please check out the documentation on sticky-accept. At least in my pre caffeine haze, that's what I'm understanding this ticket is about.

https://secure.phabricator.com/book/phabricator/article/differential_faq/

kparal closed this task as Invalid.May 30 2014, 2:18 PM
kparal claimed this task.

Oh, OK. I forgot to check the FAQ and assumed it was a bug. You're right, this is what this ticket is about. Closing as not-a-bug, Thanks for the link.

kparal added a comment.May 30 2014, 2:40 PM

As a side remark, it would be great if the revision author could reset the "Accepted" status manually, if needed. That would make sure that the ticket appears in reviewers' ticket queues again. Just saying it in the comment is not that much helpful and can be easily overlooked.

kparal added a comment.May 30 2014, 2:52 PM

As a side remark, it would be great if the revision author could reset the "Accepted" status manually, if needed.

I take it back, the manual action is available, it's called "Request Review" and it's available in the drop-down action list in these cases.

Phabricator · Built by Phacility