Can you use the modern conduit calls?

This method is frozen and will eventually be deprecated. New code should use "differential.revision.edit" instead.

Not easily. :/

Both http://www.dctrwatson.com/2013/01/jenkins-and-phabricator/ and https://github.com/uber/phabricator-jenkins-plugin use differential.createcomment. Would be a pretty large overhaul to use differential.revision.edit instead (which requires the use of PHIDs rather than just differential revision IDs).

Does this work?

diff --git a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
index 52736d6..943c3e7 100644
--- a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
@@ -49,6 +49,7 @@ final class DifferentialCreateCommentConduitAPIMethod
       ->withIDs(array($request->getValue('revision_id')))
       ->needReviewers(true)
       ->needReviewerAuthority(true)
+      ->needActiveDiffs(true)
       ->executeOne();
     if (!$revision) {
       throw new ConduitException('ERR_BAD_REVISION');

That does indeed work! Thank you so much!

Note also that differential.revision.edit accepts IDs and harbormaster.createartifact allows you to create links to build results without needing to gum up the feed with comments. We will eventually remove differential.createcomment so things will need to update sooner or later.

Also, test agents probably shouldn't be accepting revisions at all. But ¯\_(ツ)_/¯

Thanks, everybody! Appreciate it, especially on a Friday night. :-)

In T12766#225295, @epriestley wrote:

Also, test agents probably shouldn't be accepting revisions at all. But ¯\_(ツ)_/¯

That was just differential/revision/diff confusion, we're actually using diff id's, that should be the good strategy right?

The general story on Build/CI is this:

Long ago, we didn't have any formal upstream support for Build/CI. In this era, various users hacked together partial approaches, like @DctrWatson's blog post from 2013. These generally worked by leaving comments with build status and, in some cases, accepting revisions on build pass / rejecting revisions on build failure. They tended to use arc patch to apply revisions in build environments, sometimes triggered based on unusual events (other than "revision updated"), and took other approaches which weren't great but were the best things available at the time.

Since 2013, we built Harbormaster and now have formal methods for triggering builds, handing changes to CI, reporting build status, and attaching "artifacts", including links to external build results. If you use these mechanisms, you get additional integrations. For example: in Differential, reviewers are warned about running or failing builds in the comment area; and arc land warns you about running or failing builds and forces you explicitly "y" through a "Land anyway, even though this is failing?" prompt. You also get consistent triggers, consistent change handoff, etc. These integrations are generally richer than what's possible by leaving comments or taking "accept" actions, and have fewer weird side effects (including simple stuff, like leaving outdated comment clutter in the review timeline).

We recommend installs transition to these modern approaches as time allows. The old approaches won't necessarily stop working (and even if we removed "frozen" API methods, the API is modular and you could just put them back in place), but the upstream has never supported doing build integration through comments / "Accept" and if you report a bug with this to us or ask us to improve something, we'll probably say "switch to the modern stuff which we actually support" before moving forward.

(In this case, our behavior was strictly wrong, completely accidental, and trivial to fix -- but sooner or later we may make some kind of change which isn't very compatible with this older style of integration in a more fundamental way. When we do, a blog post from 2013 suggesting a set of workarounds at the time won't hold back the tides of change.)