Add behaviors to Build Plans: hold drafts, affect buildables, warn on landing, restartable, runnable
ClosedPublic
Actions

Authored by epriestley on Feb 26 2019, 5:44 PM.

Details

Reviewers

Maniphest Tasks

T11415: Would like people who can't edit Harbormaster plans to be able to run them
T13258: Plans: Harbormaster Build Plan Flags ("Does Not Block Promotion", etc)

Commits

rPd36d0efc3570: Add behaviors to Build Plans: hold drafts, affect buildables, warn on landing…

Summary

Depends on D20219. Ref T13258. Ref T11415. Installs sometimes have long-running builds or unimportant builds which they may not want to hold up drafts, affect buildable status, or warn during arc land.

Some builds have side effects (like deployment or merging) and are not idempotent. They can cause problems if restarted.

In other cases, builds are isolated and idempotent and generally safe, and it's okay for marketing interns to restart them.

To address these cases, add "Behaviors" to Build Plans:

Hold Drafts: Controls how the build affects revision promotion from "Draft".
Warn on Land: Controls the "arc land" warning.
Affects Buildable: Controls whether we care about this build when figuring out if a buildable passed or failed overall.
Restartable: Controls whether this build may restart or not.
Runnable: Allows you to weaken the requirements to run the build if you're confident it's safe to run it on arbitrary old versions of things.

NOTE: This only implements UI, none of these options actually do anything yet.

Test Plan

Mostly poked around the UI. I'll actually implement these behaviors next, and vet them more thoroughly.

Screen Shot 2019-02-26 at 9.36.03 AM.png (693×837 px, 114 KB)

Diff Detail

Repository

rP Phabricator

Branch

harbor5

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 22131
Build 30239: Run Core Tests
Build 30238: arc lint + arc unit

Event Timeline

epriestley created this revision.Feb 26 2019, 5:44 PM

Owners added a subscriber: Restricted Owners Package.Feb 26 2019, 5:44 PM

epriestley edited the summary of this revision. (Show Details)Feb 26 2019, 5:46 PM

Harbormaster completed remote builds in B22131: Diff 48269.Feb 26 2019, 5:46 PM

epriestley requested review of this revision.Feb 26 2019, 5:46 PM

Just a bunch of language nitpicks.

src/applications/harbormaster/plan/HarbormasterBuildPlanBehavior.php
126–128	All three of these are kinda hard to parse with the "do not" combined with "and/but". For all of these, replace "will be sent for review" with "will promote from 'Draft'" if you think that's clearer. "Revisions will be sent for review only after the build completes successfully."
135–136	"Revisions will be sent for review after the build completes, even if it failed."
143–144	"Revisions will be sent for review immediately, without waiting for the build to complete."
191–192	"The buildable passes after the build completes successfully."
199–200	"The buildable passes after the build completes, even if the build failed."
207–208	"The buildable passes immediately without waiting for the build to complete."
286	"even if builds have failed or are still in progress."

This revision is now accepted and ready to land.Mar 5 2019, 7:29 PM

epriestley added inline comments.Mar 6 2019, 1:32 PM

src/applications/harbormaster/plan/HarbormasterBuildPlanBehavior.php
143–144	I worry "immediately" may be unclear because a revision can be waiting on multiple builds, so even if you set this build to "Hold Drafts: Never", you may not get immediate promotions. To a lesser degree, the other language is trying to focus on this too: these settings give you "less stuff blocking promotion", not necessarily "faster / immediate promotion", since the overall behavior depends on what other builds are running. That said, I also don't like the "promote from draft" terminology very much either because I don't think we use it elsewhere in the UI very often.
207–208	(As above, no setting here can make the Buildable pass "immediate" if there are other builds running.)