Page MenuHomePhabricator

Allow demoted builds to automatically promote if builds pass after a restart
ClosedPublic

Authored by epriestley on Apr 18 2018, 12:00 AM.

Details

Summary

Ref T13124. See PHI584. When you create a draft revision and it automatically demotes to "Changes Planned + Draft" because builds fail, let it promote to "Needs Review" automatically if builds pass. Usually, this will be because someone restarted the builds and they worked the second time.

Although I'm a little wary about adding even more state transitions to the diagram in T13110#237736, I think this one is reasonably natural and not ambiguous.

Test Plan
  • Created a failing build plan with a "Throw Exception" step.
  • Created a revision which hit the build plan, saw it demote to "Changes Planned" when Harbormaster failed.
  • Edited the build plan to remove the "Throw Exception" step, restarted the build, got a pass.
  • Saw revision promote again:

I didn't exhaustively test that the other 40 state transitions still work properly, but I think the scope of this change is small enough that it's unlikely I did much collateral damage.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

src/applications/differential/editor/DifferentialTransactionEditor.php
1611

This got fixed elsewhere, it just didn't actually touch this piece of code so I never removed the comment.

This revision is now accepted and ready to land.Apr 20 2018, 5:33 PM
This revision was automatically updated to reflect the committed changes.

(Sorry this sat in my queue; I'm not sure how I missed it.)

Oh, no problem.

One possibility I vaguely recall noting is that the initial email included my comment (since I made it before undraft) so it looked a bit like an update to an existing revision instead of a new review request. It's a little tricky to change the sequence, but in an ideal world the initial email might still render the normal "Summary" stuff first, then comments below in a more chronological/timeline-like way. Not sure if that helped it slip by or not.

I usually just hit this query a few times per day: https://secure.phabricator.com/differential/query/jtnzxsifugrH/

I think this just blended into the background of those lingering diffs and my brain ignored it ¯\_(ツ)_/¯