Page MenuHomePhabricator

Continue reducing callsites to ArcanistDifferentialRevisionStatus
ClosedPublic

Authored by epriestley on Aug 4 2017, 3:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 10, 1:24 PM
Unknown Object (File)
Feb 17 2024, 6:52 AM
Unknown Object (File)
Feb 1 2024, 10:46 PM
Unknown Object (File)
Dec 27 2023, 11:44 AM
Unknown Object (File)
Dec 27 2023, 11:44 AM
Unknown Object (File)
Dec 27 2023, 11:43 AM
Unknown Object (File)
Dec 21 2023, 8:18 PM
Unknown Object (File)
Dec 21 2023, 8:17 PM
Subscribers
None

Details

Summary

Ref T2543. Further consolidates status management into DifferentialRevisionStatus.

One change I'm making here is internally renaming "CLOSED" to "PUBLISHED". The UI will continue to say "Closed", at least for now, but this should make the code more clear because we care about "is closed, exactly" vs "is any closed status (closed, abandoned, sometimes accepted)". This distinction is more obvious as isClosed() vs isPublished() than, e.g., isClosedWithExactlyTheClosedStatus() or something. I think "Published" is generally more clear, too, and more consistent with modern language (e.g., "pre-publish review" replacing "pre-commit review" to make it more clear what we mean in Git/Mercurial).

I've removed the IN_PREPARATION status since this was just earlier groundwork for "Draft" and not actually used, and under the newer plan I'm trying to just abandon ArcanistDifferentialRevisionStatus entirely (or, at least, substantially).

Test Plan
  • Viewed revisions.
  • Viewed revision list.
  • Viewed revisions linked to a task in Maniphest.
  • Viewed revision graph of dependencies in Differential.
  • Grepped for COLOR_STATUS_... constants.
  • Grepped for removed method getRevisionStatusIcon() (no callsites).
  • Grepped for removed method renderFullDescription() (one callsite, replaced with just building a TagView inline).
  • Grepped for removed method isClosedStatus() (no callsites after other changes).

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Aug 4 2017, 5:20 PM
This revision was automatically updated to reflect the committed changes.