From a2588d62e7f674fcde3388aec4d36bdb0da9ecb4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 13 Apr 2016 08:51:12 -0700 Subject: [PATCH] Minor `bin/aphlict` cleanup Summary: Ref T10697. This just improves a couple of minor `bin/aphlict` things: make argument parsing more explicit/consistent, consolidate a little bit of duplicated code. Test Plan: Ran all `bin/aphlict` commands. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10697 Differential Revision: https://secure.phabricator.com/D15698 --- ...bricatorAphlictManagementDebugWorkflow.php | 7 +- ...icatorAphlictManagementRestartWorkflow.php | 7 +- ...bricatorAphlictManagementStartWorkflow.php | 6 +- ...ricatorAphlictManagementStatusWorkflow.php | 2 +- ...abricatorAphlictManagementStopWorkflow.php | 2 +- .../PhabricatorAphlictManagementWorkflow.php | 73 +++++++++---------- 6 files changed, 48 insertions(+), 49 deletions(-) diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementDebugWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementDebugWorkflow.php index 2fda59e6fe..27459b79ad 100644 --- a/src/applications/aphlict/management/PhabricatorAphlictManagementDebugWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementDebugWorkflow.php @@ -4,17 +4,18 @@ final class PhabricatorAphlictManagementDebugWorkflow extends PhabricatorAphlictManagementWorkflow { protected function didConstruct() { - parent::didConstruct(); $this ->setName('debug') ->setSynopsis( pht( 'Start the notifications server in the foreground and print large '. - 'volumes of diagnostic information to the console.')); + 'volumes of diagnostic information to the console.')) + ->setArguments($this->getLaunchArguments()); } public function execute(PhutilArgumentParser $args) { - parent::execute($args); + $this->parseLaunchArguments($args); + $this->setDebug(true); $this->willLaunch(); diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementRestartWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementRestartWorkflow.php index 55d97a9ac5..787003e382 100644 --- a/src/applications/aphlict/management/PhabricatorAphlictManagementRestartWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementRestartWorkflow.php @@ -4,19 +4,20 @@ final class PhabricatorAphlictManagementRestartWorkflow extends PhabricatorAphlictManagementWorkflow { protected function didConstruct() { - parent::didConstruct(); $this ->setName('restart') - ->setSynopsis(pht('Stop, then start the notifications server.')); + ->setSynopsis(pht('Stop, then start the notification server.')) + ->setArguments($this->getLaunchArguments()); } public function execute(PhutilArgumentParser $args) { - parent::execute($args); + $this->parseLaunchArguments($args); $err = $this->executeStopCommand(); if ($err) { return $err; } + return $this->executeStartCommand(); } diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementStartWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementStartWorkflow.php index 4217ac5903..bd034f8c88 100644 --- a/src/applications/aphlict/management/PhabricatorAphlictManagementStartWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementStartWorkflow.php @@ -4,14 +4,14 @@ final class PhabricatorAphlictManagementStartWorkflow extends PhabricatorAphlictManagementWorkflow { protected function didConstruct() { - parent::didConstruct(); $this ->setName('start') - ->setSynopsis(pht('Start the notifications server.')); + ->setSynopsis(pht('Start the notifications server.')) + ->setArguments($this->getLaunchArguments()); } public function execute(PhutilArgumentParser $args) { - parent::execute($args); + $this->parseLaunchArguments($args); return $this->executeStartCommand(); } diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementStatusWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementStatusWorkflow.php index 85f80bfad4..c9a0768bf2 100644 --- a/src/applications/aphlict/management/PhabricatorAphlictManagementStatusWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementStatusWorkflow.php @@ -6,7 +6,7 @@ final class PhabricatorAphlictManagementStatusWorkflow protected function didConstruct() { $this ->setName('status') - ->setSynopsis(pht('Show the status of the notifications server.')) + ->setSynopsis(pht('Show the status of the notification server.')) ->setArguments(array()); } diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementStopWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementStopWorkflow.php index f685afbaab..8c88e79d86 100644 --- a/src/applications/aphlict/management/PhabricatorAphlictManagementStopWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementStopWorkflow.php @@ -6,7 +6,7 @@ final class PhabricatorAphlictManagementStopWorkflow protected function didConstruct() { $this ->setName('stop') - ->setSynopsis(pht('Stop the notifications server.')) + ->setSynopsis(pht('Stop the notification server.')) ->setArguments(array()); } diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php index 659fb80edb..2df83ea49f 100644 --- a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php @@ -7,27 +7,29 @@ abstract class PhabricatorAphlictManagementWorkflow private $clientHost; private $clientPort; - protected function didConstruct() { - $this - ->setArguments( - array( - array( - 'name' => 'client-host', - 'param' => 'hostname', - 'help' => pht('Hostname to bind to for the client server.'), - ), - array( - 'name' => 'client-port', - 'param' => 'port', - 'help' => pht('Port to bind to for the client server.'), - ), - )); + final protected function setDebug($debug) { + $this->debug = $debug; + return $this; } - public function execute(PhutilArgumentParser $args) { + protected function getLaunchArguments() { + return array( + array( + 'name' => 'client-host', + 'param' => 'hostname', + 'help' => pht('Hostname to bind to for the client server.'), + ), + array( + 'name' => 'client-port', + 'param' => 'port', + 'help' => pht('Port to bind to for the client server.'), + ), + ); + } + + protected function parseLaunchArguments(PhutilArgumentParser $args) { $this->clientHost = $args->getArg('client-host'); $this->clientPort = $args->getArg('client-port'); - return 0; } final public function getPIDPath() { @@ -86,11 +88,6 @@ abstract class PhabricatorAphlictManagementWorkflow exit(1); } - final protected function setDebug($debug) { - $this->debug = $debug; - return $this; - } - public static function requireExtensions() { self::mustHaveExtension('pcntl'); self::mustHaveExtension('posix'); @@ -146,11 +143,8 @@ abstract class PhabricatorAphlictManagementWorkflow $test_argv = $this->getServerArgv(); $test_argv[] = '--test=true'; - execx( - '%s %s %Ls', - $this->getNodeBinary(), - $this->getAphlictScriptPath(), - $test_argv); + + execx('%C', $this->getStartCommand($test_argv)); } private function getServerArgv() { @@ -189,11 +183,6 @@ abstract class PhabricatorAphlictManagementWorkflow return $server_argv; } - private function getAphlictScriptPath() { - $root = dirname(phutil_get_library_root('phabricator')); - return $root.'/support/aphlict/server/aphlict_server.js'; - } - final protected function launch() { $console = PhutilConsole::getConsole(); @@ -205,11 +194,7 @@ abstract class PhabricatorAphlictManagementWorkflow Filesystem::writeFile($this->getPIDPath(), getmypid()); } - $command = csprintf( - '%s %s %Ls', - $this->getNodeBinary(), - $this->getAphlictScriptPath(), - $this->getServerArgv()); + $command = $this->getStartCommand($this->getServerArgv()); if (!$this->debug) { declare(ticks = 1); @@ -267,7 +252,6 @@ abstract class PhabricatorAphlictManagementWorkflow fclose(STDOUT); fclose(STDERR); - $this->launch(); return 0; } @@ -325,4 +309,17 @@ abstract class PhabricatorAphlictManagementWorkflow '$PATH')); } + private function getAphlictScriptPath() { + $root = dirname(phutil_get_library_root('phabricator')); + return $root.'/support/aphlict/server/aphlict_server.js'; + } + + private function getStartCommand(array $server_argv) { + return csprintf( + '%s %s %Ls', + $this->getNodeBinary(), + $this->getAphlictScriptPath(), + $server_argv); + } + }