Page MenuHomePhabricator

Move build variables into HarbormasterBuildableInterface
ClosedPublic

Authored by hach-que on Jun 18 2014, 5:47 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 29, 2:10 PM
Unknown Object (File)
Wed, Apr 24, 10:00 PM
Unknown Object (File)
Fri, Apr 19, 3:40 AM
Unknown Object (File)
Fri, Apr 19, 3:40 AM
Unknown Object (File)
Fri, Apr 19, 3:40 AM
Unknown Object (File)
Fri, Apr 19, 3:40 AM
Unknown Object (File)
Fri, Apr 19, 3:40 AM
Unknown Object (File)
Sun, Apr 14, 4:26 AM
Subscribers

Details

Summary

Ref T1049. This moves the declaration of build variables onto HarbormasterBuildableInterface, allowing new classes implementing HarbormasterBuildableInterface to declare their own variables.

Test Plan

Implemented it on another class, saw the build variables appear.

Diff Detail

Repository
rP Phabricator
Branch
build-variables
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1213
Build 1213: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

hach-que retitled this revision from to Move build variables into HarbormasterBuildableInterface.
hach-que updated this object.
hach-que edited the test plan for this revision. (Show Details)
hach-que added a reviewer: epriestley.
hach-que edited edge metadata.

Fix missing conditional

epriestley edited edge metadata.

Minor inlines, looks good overall.

src/applications/harbormaster/storage/build/HarbormasterBuild.php
238–241

Yes, this is guaranteed.

243–245

Try:

$results = $object_variables + $results;

(Note that order is important; in $a = $b + $c, if a key is present in both $b and $c, the value in $b wins.)

256

Use loadObjects(): it's a little simpler, and it makes sure you don't get abstract subclasses back.

267

Then, this loop will just be foreach ($objects as $object) and you don't need new.

This revision now requires changes to proceed.Jun 20 2014, 1:56 AM
hach-que edited edge metadata.

Changes requested in code review

This revision is now accepted and ready to land.Jun 20 2014, 2:23 AM
hach-que updated this revision to Diff 23124.

Closed by commit rPf7f866445681 (authored by @hach-que).