Page MenuHomePhabricator

Show more detailed hints about draft revisions in the UI
ClosedPublic

Authored by epriestley on Oct 19 2017, 9:11 PM.
Tags
None
Referenced Files
F13061106: D18714.diff
Fri, Apr 19, 6:54 PM
Unknown Object (File)
Sun, Apr 7, 10:27 AM
Unknown Object (File)
Fri, Apr 5, 10:53 PM
Unknown Object (File)
Thu, Apr 4, 11:18 PM
Unknown Object (File)
Sat, Mar 30, 5:17 AM
Unknown Object (File)
Fri, Mar 29, 1:24 AM
Unknown Object (File)
Thu, Mar 28, 2:41 PM
Unknown Object (File)
Mar 2 2024, 4:15 AM
Subscribers
None

Details

Summary

Ref T2543. When revisions are in the draft state, tell the user what we're waiting for or why they aren't moving forward.

Test Plan

Screen Shot 2017-10-19 at 2.07.22 PM.png (1×1 px, 216 KB)

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Otherwise looks good.

src/applications/differential/customfield/DifferentialDraftField.php
42–48

Maybe we should invert this and enumerate the non-blocking states? (Looks like there are 4 of those vs 5 of these).

Or add a HarbormasterBuildStatus::getFailedStatusConstants() to make it more likely that this code will continue to work if we add more failed states?

This revision is now accepted and ready to land.Oct 19 2017, 9:26 PM

I'm think I'm going to leave this junk in place for now -- although it likely will get refactored in the future, I don't think these changes make it much harder to refactor and I'm not sure what it's going to refactor into yet. There are a couple of open tasks somewhere about adding 30 new weird build statuses, and I don't think we'll actually support those as first-class statuses, but it's possible that build statuses will become fully user-definable. If they do, the refactoring looks different than if we stay close to this model.

There's also some crudeness in the messaging here, e.g. "Paused" will say "these builds failed: ..." and should probably be refined at some point, but I don't want to go crazy and give every status combination a bunch of custom strings before this feature even works.

This revision was automatically updated to reflect the committed changes.

And largely I'm just trying to ship some version of this this week since things have been mired a bit recently in all the ngrams/rate-limit stuff.