Arcanist / Conduit - stop using deprecated diffusion.getcommits
ClosedPublic
Actions

Authored by btrahan on Feb 3 2015, 10:31 PM.

Details

Reviewers

epriestley

Maniphest Tasks

T7113: Remove deprecated diffusion.getcommits calls from Arcanist

Commits

rARC5c20df181875: Arcanist / Conduit - stop using deprecated diffusion.getcommits

Summary

Fixes T7113. This one was a bit trickier than others as the API output changed a bit. In particular, there is no "errors" emitted so much as the result set just doesn't include the answer if things are garbage. Ergo, check the "identifier map" to either check for diff existence or to lookup the phid to grab the actual diff data from the "data" part of the result.

Test Plan

called arc backout D11665 and got some working output...!

Diff Detail

Repository

rARC Arcanist

Branch

T7113

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 4307
Build 4320: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

btrahan updated this revision to Diff 28078.Feb 3 2015, 10:31 PM

btrahan added a task: T7113: Remove deprecated diffusion.getcommits calls from Arcanist.

btrahan retitled this revision from to Arcanist / Conduit - stop using deprecated diffusion.getcommits.

btrahan updated this object.

btrahan edited the test plan for this revision. (Show Details)

btrahan added a reviewer: epriestley.

Herald added subscribers: epriestley, Korvin. · View Herald TranscriptFeb 3 2015, 10:31 PM

epriestley accepted this revision.Feb 3 2015, 10:32 PM

epriestley edited edge metadata.

This revision is now accepted and ready to land.Feb 3 2015, 10:32 PM

Closed by commit rARC5c20df181875: Arcanist / Conduit - stop using deprecated diffusion.getcommits (authored by btrahan, committed by btrahan). · Explain WhyFeb 3 2015, 10:33 PM

This revision was automatically updated to reflect the committed changes.

epriestley added inline comments.Feb 21 2015, 12:37 AM

src/workflow/ArcanistWorkflow.php
1015–1016	The old version of this was totally unreadable and neither of us caught it, but this actually inverted the logic. The intent here is to return false ("do not amend") if the commit is known. That is: do not amend published commits. This will be fixed in a second (with the fix for T7344).