Page MenuHomePhabricator

Arcanist - add revision data to TYPE_LAND_WILLPUSHREVISION event
ClosedPublic

Authored by btrahan on Mar 10 2014, 8:50 PM.
Tags
None
Referenced Files
F15483726: D8484.id20099.diff
Wed, Apr 9, 2:00 PM
F15480018: D8484.id.diff
Tue, Apr 8, 11:01 AM
F15479790: D8484.id20146.diff
Tue, Apr 8, 9:34 AM
F15478967: D8484.id20145.diff
Tue, Apr 8, 4:25 AM
F15476687: D8484.diff
Mon, Apr 7, 8:41 AM
F15465695: D8484.diff
Wed, Apr 2, 10:07 PM
F15444620: D8484.id20146.diff
Thu, Mar 27, 10:24 AM
F15431817: D8484.id20099.diff
Mon, Mar 24, 2:38 PM

Details

Summary

pretty straight-forward stuff here. Note that in other events we use "fields" or "specification" rather than "revision"; I think "revision" is best particularly in this context where it is in fact the revision being landed.

Fixes T4565.

Test Plan

php -l

Diff Detail

Repository
rARC Arcanist
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley edited edge metadata.

Hmm.. how about we expose getRevision() as a public method instead? Then the callee can access it by grabbing the 'workflow' from the event (which will be this object) and calling getRevision() on it. I think that's a little cleaner / more general / more future-proof.

One thing which occurs to me is that you may be landing multiple revisions, especially in the future. So maybe a better implementation would be:

public function getRevisions() {
  return array($this->revision);
}

...so we don't back ourselves into a corner if we get better about letting arc land cascade through an entire branch or land multiple known revisions.

This revision now requires changes to proceed.Mar 11 2014, 8:13 PM
btrahan edited edge metadata.

Not passing the revision and instead adding "getRevisionDict" -- I felt like "Dict" was good to add to the member method as I would assume a revision object otherwise.

epriestley edited edge metadata.

Seems reasonable.

This revision is now accepted and ready to land.Mar 11 2014, 10:20 PM
btrahan updated this revision to Diff 20146.

Closed by commit rARC37dac6113168 (authored by @btrahan).