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
Unknown Object (File)
Wed, Nov 27, 1:06 AM
Unknown Object (File)
Mon, Nov 25, 11:39 PM
Unknown Object (File)
Sat, Nov 23, 2:29 AM
Unknown Object (File)
Tue, Nov 19, 1:48 PM
Unknown Object (File)
Thu, Nov 14, 5:49 PM
Unknown Object (File)
Sun, Nov 10, 6:12 PM
Unknown Object (File)
Wed, Nov 6, 9:53 PM
Unknown Object (File)
Oct 29 2024, 3:50 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).