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
F13242531: D8484.diff
Thu, May 23, 2:51 AM
F13220670: D8484.diff
Sun, May 19, 1:51 AM
F13202681: D8484.diff
Tue, May 14, 11:00 PM
F13199512: D8484.id20145.diff
Mon, May 13, 3:36 PM
F13199511: D8484.id20146.diff
Mon, May 13, 3:36 PM
F13199510: D8484.id20099.diff
Mon, May 13, 3:36 PM
F13199503: D8484.id.diff
Mon, May 13, 3:33 PM
F13199498: D8484.diff
Mon, May 13, 3:29 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
Branch
T4565
Lint
Lint Passed
Unit
No Test Coverage

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).