Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Rename "arc mark-committed" to "arc close-revision"

Summary:
  - Replace SVN-specific language with VCS-agnostic language.
  - Add new "arc close-revision", works exactly like "arc mark-committed" but with agnostic language.
  - Use status constants, not status strings.
  - Mark "arc mark-committed" deprecated.
  - Remove deprecated "arc merge".

Test Plan: Ran "arc mark-committed", "arc close-revision".

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran, Makinde

Maniphest Tasks: T909

Differential Revision: https://secure.phabricator.com/D2244
  • Loading branch information...
commit da4b6b5799f01e9a61564f8013c2d04bf37ec03b 1 parent c320e09
@epriestley epriestley authored
View
4 src/__phutil_library_map__.php
@@ -21,6 +21,7 @@
'ArcanistCapabilityNotSupportedException' => 'workflow/exception/notsupported',
'ArcanistChooseInvalidRevisionException' => 'exception',
'ArcanistChooseNoRevisionsException' => 'exception',
+ 'ArcanistCloseRevisionWorkflow' => 'workflow/close-revision',
'ArcanistCloseWorkflow' => 'workflow/close',
'ArcanistCommitWorkflow' => 'workflow/commit',
'ArcanistConduitLinter' => 'lint/linter/conduit',
@@ -70,7 +71,6 @@
'ArcanistMercurialAPI' => 'repository/api/mercurial',
'ArcanistMercurialParser' => 'repository/parser/mercurial',
'ArcanistMercurialParserTestCase' => 'repository/parser/mercurial/__tests__',
- 'ArcanistMergeWorkflow' => 'workflow/merge',
'ArcanistNoEffectException' => 'exception/usage/noeffect',
'ArcanistNoEngineException' => 'exception/usage/noengine',
'ArcanistNoLintLinter' => 'lint/linter/nolint',
@@ -127,6 +127,7 @@
'ArcanistBranchWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistBundleTestCase' => 'ArcanistPhutilTestCase',
'ArcanistCallConduitWorkflow' => 'ArcanistBaseWorkflow',
+ 'ArcanistCloseRevisionWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistCloseWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistCommitWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistConduitLinter' => 'ArcanistLinter',
@@ -154,7 +155,6 @@
'ArcanistMarkCommittedWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistMercurialAPI' => 'ArcanistRepositoryAPI',
'ArcanistMercurialParserTestCase' => 'ArcanistPhutilTestCase',
- 'ArcanistMergeWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistNoEffectException' => 'ArcanistUsageException',
'ArcanistNoEngineException' => 'ArcanistUsageException',
'ArcanistNoLintLinter' => 'ArcanistLinter',
View
5 src/differential/constants/revisionstatus/ArcanistDifferentialRevisionStatus.php
@@ -21,7 +21,8 @@
const NEEDS_REVIEW = 0;
const NEEDS_REVISION = 1;
const ACCEPTED = 2;
- const COMMITTED = 3;
+ const COMMITTED = 3; // TODO: Remove.
+ const CLOSED = 3; // NOTE: Duplicate of "COMMITTED".
const ABANDONED = 4;
public static function getNameForRevisionStatus($status) {
@@ -29,7 +30,7 @@ public static function getNameForRevisionStatus($status) {
self::NEEDS_REVIEW => 'Needs Review',
self::NEEDS_REVISION => 'Needs Revision',
self::ACCEPTED => 'Accepted',
- self::COMMITTED => 'Committed',
+ self::CLOSED => 'Closed',
self::ABANDONED => 'Abandoned',
);
View
2  src/workflow/amend/ArcanistAmendWorkflow.php
@@ -176,7 +176,7 @@ public function run() {
$repository_api->amendGitHeadCommit($message);
$mark_workflow = $this->buildChildWorkflow(
- 'mark-committed',
+ 'close-revision',
array(
'--finalize',
$revision_id,
View
174 src/workflow/close-revision/ArcanistCloseRevisionWorkflow.php
@@ -0,0 +1,174 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Explicitly closes Differential revisions.
+ *
+ * @group workflow
+ */
+final class ArcanistCloseRevisionWorkflow extends ArcanistBaseWorkflow {
+
+ public function getCommandSynopses() {
+ return phutil_console_format(<<<EOTEXT
+ **close-revision** [__options__] __revision__
+EOTEXT
+ );
+ }
+
+ public function getCommandHelp() {
+ return phutil_console_format(<<<EOTEXT
+ Supports: git, hg, svn
+ Close a revision which has been committed (svn) or pushed (git, hg).
+ You should not normally need to do this: arc commit (svn), arc amend
+ (git), arc land (git), or repository tracking on the master remote
+ repository should do it for you. However, if these mechanisms have
+ failed for some reason you can use this command to manually change a
+ revision status from "Accepted" to "Closed".
+EOTEXT
+ );
+ }
+
+ public function getArguments() {
+ return array(
+ 'finalize' => array(
+ 'help' =>
+ "Close only if the repository is untracked and the revision is ".
+ "accepted. Continue even if the close can't happen. This is a soft ".
+ "version of 'close-revision' used by other workflows.",
+ ),
+ 'quiet' => array(
+ 'help' => 'Do not print a success message.',
+ ),
+ '*' => 'revision',
+ );
+ }
+
+ public function requiresConduit() {
+ return true;
+ }
+
+ public function requiresAuthentication() {
+ return true;
+ }
+
+ public function requiresRepositoryAPI() {
+ // NOTE: Technically we only use this to generate the right message at
+ // the end, and you can even get the wrong message (e.g., if you run
+ // "arc close-revision D123" from a git repository, but D123 is an SVN
+ // revision). We could be smarter about this, but it's just display fluff.
+ return true;
+ }
+
+ public function run() {
+ $is_finalize = $this->getArgument('finalize');
+
+ $conduit = $this->getConduit();
+
+ $revision_list = $this->getArgument('revision', array());
+ if (!$revision_list) {
+ throw new ArcanistUsageException(
+ "close-revision requires a revision number.");
+ }
+ if (count($revision_list) != 1) {
+ throw new ArcanistUsageException(
+ "close-revision requires exactly one revision.");
+ }
+ $revision_id = reset($revision_list);
+ $revision_id = $this->normalizeRevisionID($revision_id);
+
+ $revision = null;
+ try {
+ $revision = $conduit->callMethodSynchronous(
+ 'differential.getrevision',
+ array(
+ 'revision_id' => $revision_id,
+ )
+ );
+ } catch (Exception $ex) {
+ if (!$is_finalize) {
+ throw new ArcanistUsageException(
+ "Revision D{$revision_id} does not exist."
+ );
+ }
+ }
+
+ $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
+ $status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
+
+ if (!$is_finalize && $revision['status'] != $status_accepted) {
+ throw new ArcanistUsageException(
+ "Revision D{$revision_id} can not be closed. You can only close ".
+ "revisions which have been 'accepted'.");
+ }
+
+ if ($revision) {
+ if (!$is_finalize && $revision['authorPHID'] != $this->getUserPHID()) {
+ $prompt = "You are not the author of revision D{$revision_id}, ".
+ 'are you sure you want to close it?';
+ if (!phutil_console_confirm($prompt)) {
+ throw new ArcanistUserAbortException();
+ }
+ }
+
+ $actually_close = true;
+ if ($is_finalize) {
+ $project_info = $conduit->callMethodSynchronous(
+ 'arcanist.projectinfo',
+ array(
+ 'name' => $this->getWorkingCopy()->getProjectID(),
+ ));
+ if ($project_info['tracked'] ||
+ $revision['status'] != $status_accepted) {
+ $actually_close = false;
+ }
+ }
+ if ($actually_close) {
+ $revision_name = $revision['title'];
+
+ echo "Closing revision D{$revision_id} '{$revision_name}'...\n";
+
+ $conduit->callMethodSynchronous(
+ 'differential.markcommitted',
+ array(
+ 'revision_id' => $revision_id,
+ ));
+ }
+ }
+
+ $status = $revision['status'];
+ if ($status == $status_accepted || $status == $status_closed) {
+ // If this has already been attached to commits, don't show the
+ // "you can push this commit" message since we know it's been pushed
+ // already.
+ $is_finalized = empty($revision['commits']);
+ } else {
+ $is_finalized = false;
+ }
+
+ if (!$this->getArgument('quiet')) {
+ if ($is_finalized) {
+ $message = $this->getRepositoryAPI()->getFinalizedRevisionMessage();
+ echo phutil_console_wrap($message)."\n";
+ } else {
+ echo "Done.\n";
+ }
+ }
+
+ return 0;
+ }
+}
View
4 src/workflow/merge/__init__.php → src/workflow/close-revision/__init__.php
@@ -6,10 +6,12 @@
+phutil_require_module('arcanist', 'differential/constants/revisionstatus');
phutil_require_module('arcanist', 'exception/usage');
+phutil_require_module('arcanist', 'exception/usage/userabort');
phutil_require_module('arcanist', 'workflow/base');
phutil_require_module('phutil', 'console');
-phutil_require_source('ArcanistMergeWorkflow.php');
+phutil_require_source('ArcanistCloseRevisionWorkflow.php');
View
2  src/workflow/commit/ArcanistCommitWorkflow.php
@@ -169,7 +169,7 @@ public function run() {
}
$mark_workflow = $this->buildChildWorkflow(
- 'mark-committed',
+ 'close-revision',
array(
'--finalize',
$revision_id,
View
2  src/workflow/land/ArcanistLandWorkflow.php
@@ -290,7 +290,7 @@ public function run() {
}
$mark_workflow = $this->buildChildWorkflow(
- 'mark-committed',
+ 'close-revision',
array(
'--finalize',
'--quiet',
View
123 src/workflow/mark-committed/ArcanistMarkCommittedWorkflow.php
@@ -17,44 +17,27 @@
*/
/**
- * Explicitly marks Differential revisions as "Committed".
- *
* @group workflow
*/
final class ArcanistMarkCommittedWorkflow extends ArcanistBaseWorkflow {
public function getCommandSynopses() {
return phutil_console_format(<<<EOTEXT
- **mark-committed** __revision__
+ **mark-committed** (DEPRECATED)
EOTEXT
);
}
public function getCommandHelp() {
return phutil_console_format(<<<EOTEXT
- Supports: git, svn
- Manually mark a revision as committed. You should not normally need
- to do this; arc commit (svn), arc amend (git), arc merge (git, hg) or
- repository tracking on the master remote repository should do it for
- you. However, if these mechanisms have failed for some reason you can
- use this command to manually change a revision status from "Accepted"
- to "Committed".
+ Deprecated. Moved to "close-revision".
EOTEXT
);
}
public function getArguments() {
return array(
- 'finalize' => array(
- 'help' =>
- "Mark committed only if the repository is untracked and the ".
- "revision is accepted. Continue even if the mark can't happen. This ".
- "is a soft version of 'mark-committed' used by other workflows.",
- ),
- 'quiet' => array(
- 'help' => 'Do not print a success message.',
- ),
- '*' => 'revision',
+ '*' => 'deprecated',
);
}
@@ -67,106 +50,12 @@ public function requiresAuthentication() {
}
public function requiresRepositoryAPI() {
- // NOTE: Technically we only use this to generate the right message at
- // the end, and you can even get the wrong message (e.g., if you run
- // "arc mark-committed D123" from a git repository, but D123 is an SVN
- // revision). We could be smarter about this, but it's just display fluff.
return true;
}
public function run() {
- $is_finalize = $this->getArgument('finalize');
-
- $conduit = $this->getConduit();
-
- $revision_list = $this->getArgument('revision', array());
- if (!$revision_list) {
- throw new ArcanistUsageException(
- "mark-committed requires a revision number.");
- }
- if (count($revision_list) != 1) {
- throw new ArcanistUsageException(
- "mark-committed requires exactly one revision.");
- }
- $revision_id = reset($revision_list);
- $revision_id = $this->normalizeRevisionID($revision_id);
-
- $revision = null;
- try {
- $revision = $conduit->callMethodSynchronous(
- 'differential.getrevision',
- array(
- 'revision_id' => $revision_id,
- )
- );
- } catch (Exception $ex) {
- if (!$is_finalize) {
- throw new ArcanistUsageException(
- "Revision D{$revision_id} does not exist."
- );
- }
- }
-
- if (!$is_finalize && $revision['statusName'] != 'Accepted') {
- throw new ArcanistUsageException(
- "Revision D{$revision_id} is not committable. You can only mark ".
- "revisions which have been 'accepted' as committed."
- );
- }
-
- if ($revision) {
- if (!$is_finalize && $revision['authorPHID'] != $this->getUserPHID()) {
- $prompt = "You are not the author of revision D{$revision_id}, ".
- 'are you sure you want to mark it committed?';
- if (!phutil_console_confirm($prompt)) {
- throw new ArcanistUserAbortException();
- }
- }
-
- $actually_mark = true;
- if ($is_finalize) {
- $project_info = $conduit->callMethodSynchronous(
- 'arcanist.projectinfo',
- array(
- 'name' => $this->getWorkingCopy()->getProjectID(),
- ));
- if ($project_info['tracked'] || $revision['statusName'] != 'Accepted') {
- $actually_mark = false;
- }
- }
- if ($actually_mark) {
- $revision_name = $revision['title'];
-
- echo "Marking revision D{$revision_id} '{$revision_name}' ".
- "committed...\n";
-
- $conduit->callMethodSynchronous(
- 'differential.markcommitted',
- array(
- 'revision_id' => $revision_id,
- ));
- }
- }
-
- $status = $revision['statusName'];
- if ($status == 'Accepted' || $status == 'Committed') {
- // If this has already been attached to commits, don't show the
- // "you can push this commit" message since we know it's been committed
- // already.
- $is_finalized = empty($revision['commits']);
- } else {
- $is_finalized = false;
- }
-
- if (!$this->getArgument('quiet')) {
- if ($is_finalized) {
- $message = $this->getRepositoryAPI()->getFinalizedRevisionMessage();
- echo phutil_console_wrap($message)."\n";
- } else {
- echo "Done.\n";
- }
- }
-
- return 0;
+ throw new ArcanistUsageException(
+ "'arc mark-committed' is now 'arc close-revision' (because ".
+ "'mark-committed' only really made sense under SVN).");
}
}
View
1  src/workflow/mark-committed/__init__.php
@@ -7,7 +7,6 @@
phutil_require_module('arcanist', 'exception/usage');
-phutil_require_module('arcanist', 'exception/usage/userabort');
phutil_require_module('arcanist', 'workflow/base');
phutil_require_module('phutil', 'console');
View
65 src/workflow/merge/ArcanistMergeWorkflow.php
@@ -1,65 +0,0 @@
-<?php
-
-/*
- * Copyright 2012 Facebook, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/**
- * Deprecated.
- *
- * TODO: Remove soon, once git users have had a chance to see the "use land
- * instead" message.
- *
- * @group workflow
- */
-final class ArcanistMergeWorkflow extends ArcanistBaseWorkflow {
-
- public function getCommandSynopses() {
- return phutil_console_format(<<<EOTEXT
- **merge**
-EOTEXT
- );
- }
-
- public function getCommandHelp() {
- return phutil_console_format(<<<EOTEXT
- Deprecated.
-EOTEXT
- );
- }
-
- public function requiresRepositoryAPI() {
- return true;
- }
-
- public function getArguments() {
- return array(
- '*' => 'ignored',
- );
- }
-
- public function run() {
- $repository_api = $this->getRepositoryAPI();
-
- if ($repository_api instanceof ArcanistGitAPI) {
- throw new ArcanistUsageException(
- "'arc merge' no longer supports git. Use ".
- "'arc land --keep-branch --hold --merge <feature_branch>' instead.");
- }
-
- throw new ArcanistUsageException('arc merge is no longer supported.');
- }
-
-}
Please sign in to comment.
Something went wrong with that request. Please try again.