Page MenuHomePhabricator
Differential D18837

Make "arc land" accommodate a minor API change in Harbormaster build statuses
ClosedPublic
Actions

Authored by epriestley on Dec 22 2017, 5:18 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 3, 7:28 AM
Unknown Object (File)
Thu, Apr 25, 6:32 AM
Unknown Object (File)
Thu, Apr 25, 1:34 AM
Unknown Object (File)
Fri, Apr 19, 7:03 PM
Unknown Object (File)
Tue, Apr 16, 5:20 PM
Unknown Object (File)
Sun, Apr 14, 12:53 PM
Unknown Object (File)
Sat, Apr 13, 9:21 AM
Unknown Object (File)
Sat, Apr 13, 2:42 AM
View All Files
Subscribers
None

Details

Reviewers
amckinley
Commits
rARCc784b569202f: Make "arc land" accommodate a minor API change in Harbormaster build statuses
Summary

See PHI261. Currently "arc land" shows every build staus (passed, failed, building, etc) as yellow. Intended behavior is that passed builds are green, failed builds are red, and so on.

This is because of an unintended API change a while ago in D16356. Since the only impact was a cosmetic color issue, this escaped notice until now.

Additionally, try to use the modern harbormaster.build.search if it is available.

Test Plan
  • Ran arc land with running builds, got reasonable coloration.
  • Faked the new method not being available, still got sensible behavior from the old method.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley created this revision.Dec 22 2017, 5:18 PM
Harbormaster completed remote builds in B18970: Diff 45193.Dec 22 2017, 5:19 PM
epriestley requested review of this revision.Dec 22 2017, 5:19 PM
epriestley updated this revision to Diff 45194.Dec 22 2017, 5:29 PM
  • Various minor fixes after actually executing the test plan.
Harbormaster completed remote builds in B18971: Diff 45194.Dec 22 2017, 5:30 PM
epriestley added a comment.Dec 22 2017, 6:07 PM

Specifically, the old API result looked like this:

'buildStatus' => 'passed'

The newer one looks like this:

'buildStatus' => array(
  'value' => 'passed',
  'name' => 'Passed',
)

That caused the switch() (see inline) to always hit the default case and go yellow.

src/workflow/ArcanistLandWorkflow.php
1429

This became switch (array(...)) { ... }.

amckinley accepted this revision.Dec 23 2017, 5:27 PM
This revision is now accepted and ready to land.Dec 23 2017, 5:27 PM
Closed by commit rARCc784b569202f: Make "arc land" accommodate a minor API change in Harbormaster build statuses (authored by epriestley). · Explain WhyDec 23 2017, 7:39 PM
This revision was automatically updated to reflect the committed changes.
epriestley mentioned this in rARC3d06bd4c568c: Fix a minor 'buildableStatus' error in "arc land".Dec 26 2017, 4:33 PM

Revision Contents
Changeset List

PathSize
src/
2 lines
ref/
115 lines
workflow/
66 lines

Diff 45207

View Options

src/__phutil_library_map__.php

Loading...
View Options

src/ref/ArcanistBuildRef.php

Loading...
View Options

src/workflow/ArcanistLandWorkflow.php

Loading...
Phabricator · Built by Phacility