Page MenuHomePhabricator

Should be able to accept a revision again.
Closed, ResolvedPublic

Description

I recently experienced the following workflow:

  1. I write a revision.
  2. It is accepted.
  3. The acceptor, impatient, commandeers the revision.
  4. The commandeer-er makes more changes.
  5. I can't accept, because it's already been accepted, but the sender can't land!

The workaround is to Request Changes, and then immediately Accept, but this seems excessive.

Event Timeline

tyrone.nicholas raised the priority of this task from to Needs Triage.
tyrone.nicholas updated the task description. (Show Details)
tyrone.nicholas added a project: Facebook.
tyrone.nicholas added a subscriber: tyrone.nicholas.
epriestley added subscribers: Unknown Object (MLST), epriestley.Feb 14 2014, 2:03 PM

I think this is mostly due to Facebook being about four months out of date. This workflow is completely different at HEAD, and we track per-reviewer status and allow multiple accepts, among other things. T1279 discusses the changes and their motivation.

T1279 still has a few rough edges which we're ironing out, so this particular workflow may have some room for improvement at HEAD (T1279 has some discussion), but "you can't accept an already-accepted revision" is no longer a rule.

epriestley closed this task as Resolved.Feb 14 2014, 2:09 PM
epriestley claimed this task.

I'm going to close this since I think there's nothing else we can do on our side: I believe this workflow already works at HEAD, and we don't have much influence on Facebook's maintenance policies. Any changes we made wouldn't matter anyway, since Facebook would need to update and couldn't feasibly cherry-pick them (they would be on the other side of T1279 and T2222, both of which performed a large number of data migrations, and wildly impractical to apply via cherry-picking).

If I'm misunderstanding and you're already at or near HEAD and this still doesn't work, feel free to reopen this -- we're happy to improve this workflow if there are deficiencies in the modern version of it. (And we definitely agree the old workflows weren't great in a lot of cases like this one, which is why we did T1279.) Thanks for the report, in any case!