Page MenuHomePhabricator
Create Task
Maniphest T7311

Arc land includes reviewers which have not accepted the revision
Closed, WontfixPublic
Actions

Description

how to reproduce

  • create a new diff as user1
  • add 2 reviewers: user2 and user3
  • user2 accepts the revision
  • user1 merges the diff with "arc land"

result:
git log" shows "Reviewed by: user2, user3"

problem
user3 did not agree to the patch and maybe even has concerns

solution
do not add users to reviewed by if they haven't accepted the revision or add a "(unaccepted)" hint after the username.

Related Objects

Event Timeline

vinzent created this task.Feb 18 2015, 8:36 AM
vinzent raised the priority of this task from to Needs Triage.
vinzent updated the task description. (Show Details)
vinzent added a project: Arcanist.
vinzent added a subscriber: vinzent.
epriestley added a subscriber: epriestley.Feb 18 2015, 2:36 PM

Are you sure it says "Reviewed by: user2, user3", not "Reviewers: user2, user3"?

The expectation is that there are two fields, "Reviewers" and "Reviewed by". The "Reviewers" field lists everyone who was asked to review the revision. The "Reviewed by" field lists users who actually did. We retain both fields because "Reviewers" can be helpful independent of "Reviewed by".

I can not reproduce this issue as described. For example, in rPebcab8ed, I added btrahan and joshuaspence as reviewers. Only btrahan accepted it, and only btrahan appears as "Reviewed by". Both users appear under "Reviewers".

chad closed this task as Invalid.Feb 18 2015, 10:48 PM
chad claimed this task.
chad added a subscriber: chad.

Doesn't sound like this reproduces at HEAD.

vinzent changed the task status from Invalid to Wontfix.Feb 19 2015, 6:36 AM

OMG, sorry for wasting your time! epristley is actually right. I've somehow managed to mix up "Reviewers" and "Reviewed By" in the log....

Phabricator · Built by Phacility