Page MenuHomePhabricator

Deprecate "PhutilExecPassthru->execute()" in favor of "PhutilExecPassthru->resolve()"
Closed, ResolvedPublic

Description

Currently, PhutilExecPassthru exposes both resolve() and execute(). They largely do the same thing and have the same return value and error semantics, but calling execute() directly skips a bunch of "Future" behavior, including integration with "ServiceProfiler". In particular, passthru commands that are invoked with execute() do not appear in --trace output.

I believe this is leftover legacy behavior from the time when PhutilExecPassthru was not a Future.

I plan to:

  • Make all existing callsites invoke resolve(), not execute().
  • Rename execute() and make it private.
  • Leave execute() as a public API with a deprecation warning to avoid breaking any existing code too abruptly. The class was explicitly documented as exposing an execute() API so I don't want to pull the rug out from any callers in the wild. This can be removed after some time.

Revisions and Commits

Restricted Differential Revision
Restricted Differential Revision

Event Timeline

epriestley created this task.
epriestley added a revision: Restricted Differential Revision.Jul 21 2021, 4:54 PM
epriestley added a revision: Restricted Differential Revision.Jul 21 2021, 5:11 PM
epriestley closed this task as Resolved by committing Restricted Diffusion Commit.Jul 21 2021, 5:21 PM
epriestley added a commit: Restricted Diffusion Commit.
epriestley added a commit: Restricted Diffusion Commit.