Skip to content

Commit 20a5c9b

Browse files
author
epriestley
committed
Use "closed", not "committed", in Differential
Summary: "Committed" is SVN-specific language, and confusing in Git and Mercurial. Use neutral language instead. Test Plan: Inspection. Reviewers: btrahan, Makinde, vrana, jungejason Reviewed By: vrana CC: aran Maniphest Tasks: T909 Differential Revision: https://secure.phabricator.com/D2087
1 parent 42b1c73 commit 20a5c9b

27 files changed

+188
-82
lines changed

scripts/repository/undo_commits.php

+5-5
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
2. Put all their identifiers (commit hashes in git/hg, revision
3737
numbers in svn) into a file, one per line.
3838
3. Pipe that file into this script with relevant arguments.
39-
4. Revisions marked "committed" by those commits will be
39+
4. Revisions marked "closed" by those commits will be
4040
restored to their previous state.
4141
4242
EOSYNOPSIS
@@ -90,7 +90,7 @@
9090

9191
$commit_phids = mpull($commits, 'getPHID', 'getPHID');
9292

93-
echo "Looking up revisions marked 'committed' by these commits...\n";
93+
echo "Looking up revisions marked 'closed' by these commits...\n";
9494
$revision_ids = queryfx_all(
9595
id(new DifferentialRevision())->establishConnection('r'),
9696
'SELECT DISTINCT revisionID from %T WHERE commitPHID IN (%Ls)',
@@ -104,7 +104,7 @@
104104
return;
105105
}
106106

107-
$status_committed = ArcanistDifferentialRevisionStatus::COMMITTED;
107+
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
108108

109109
$revisions = array();
110110
$map = array();
@@ -114,8 +114,8 @@
114114
echo "Assessing revision D{$revision_id}...\n";
115115
$revision = id(new DifferentialRevision())->load($revision_id);
116116

117-
if ($revision->getStatus() != $status_committed) {
118-
echo "Revision is not 'committed', skipping.\n";
117+
if ($revision->getStatus() != $status_closed) {
118+
echo "Revision is not 'closed', skipping.\n";
119119
}
120120

121121
$assoc_commits = queryfx_all(

src/__phutil_library_map__.php

+2
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@
124124
'ConduitAPI_conduit_ping_Method' => 'applications/conduit/method/conduit/ping',
125125
'ConduitAPI_daemon_launched_Method' => 'applications/conduit/method/daemon/launched',
126126
'ConduitAPI_daemon_log_Method' => 'applications/conduit/method/daemon/log',
127+
'ConduitAPI_differential_close_Method' => 'applications/conduit/method/differential/close',
127128
'ConduitAPI_differential_createcomment_Method' => 'applications/conduit/method/differential/createcomment',
128129
'ConduitAPI_differential_creatediff_Method' => 'applications/conduit/method/differential/creatediff',
129130
'ConduitAPI_differential_createrevision_Method' => 'applications/conduit/method/differential/createrevision',
@@ -1116,6 +1117,7 @@
11161117
'ConduitAPI_conduit_ping_Method' => 'ConduitAPIMethod',
11171118
'ConduitAPI_daemon_launched_Method' => 'ConduitAPIMethod',
11181119
'ConduitAPI_daemon_log_Method' => 'ConduitAPIMethod',
1120+
'ConduitAPI_differential_close_Method' => 'ConduitAPIMethod',
11191121
'ConduitAPI_differential_createcomment_Method' => 'ConduitAPIMethod',
11201122
'ConduitAPI_differential_creatediff_Method' => 'ConduitAPIMethod',
11211123
'ConduitAPI_differential_createrevision_Method' => 'ConduitAPIMethod',
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
3+
/*
4+
* Copyright 2012 Facebook, Inc.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
/**
20+
* @group conduit
21+
*/
22+
final class ConduitAPI_differential_close_Method
23+
extends ConduitAPIMethod {
24+
25+
public function getMethodDescription() {
26+
return "Close a Differential revision.";
27+
}
28+
29+
public function defineParamTypes() {
30+
return array(
31+
'revisionID' => 'required int',
32+
);
33+
}
34+
35+
public function defineReturnType() {
36+
return 'void';
37+
}
38+
39+
public function defineErrorTypes() {
40+
return array(
41+
'ERR_NOT_FOUND' => 'Revision was not found.',
42+
);
43+
}
44+
45+
protected function execute(ConduitAPIRequest $request) {
46+
$id = $request->getValue('revisionID');
47+
48+
$revision = id(new DifferentialRevision())->load($id);
49+
if (!$revision) {
50+
throw new ConduitException('ERR_NOT_FOUND');
51+
}
52+
53+
if ($revision->getStatus() == ArcanistDifferentialRevisionStatus::CLOSED) {
54+
// This can occur if someone runs 'close-revision' and hits a race, or
55+
// they have a remote hook installed but don't have the
56+
// 'remote_hook_installed' flag set, or similar. In any case, just treat
57+
// it as a no-op rather than adding another "X closed this revision"
58+
// message to the revision comments.
59+
return;
60+
}
61+
62+
$revision->loadRelationships();
63+
64+
$editor = new DifferentialCommentEditor(
65+
$revision,
66+
$request->getUser()->getPHID(),
67+
DifferentialAction::ACTION_CLOSE);
68+
$editor->save();
69+
70+
$revision->setStatus(ArcanistDifferentialRevisionStatus::CLOSED);
71+
$revision->setDateCommitted(time());
72+
$revision->save();
73+
74+
return;
75+
}
76+
77+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
/**
3+
* This file is automatically generated. Lint this module to rebuild it.
4+
* @generated
5+
*/
6+
7+
8+
9+
phutil_require_module('arcanist', 'differential/constants/revisionstatus');
10+
11+
phutil_require_module('phabricator', 'applications/conduit/method/base');
12+
phutil_require_module('phabricator', 'applications/conduit/protocol/exception');
13+
phutil_require_module('phabricator', 'applications/differential/constants/action');
14+
phutil_require_module('phabricator', 'applications/differential/editor/comment');
15+
phutil_require_module('phabricator', 'applications/differential/storage/revision');
16+
17+
phutil_require_module('phutil', 'utils');
18+
19+
20+
phutil_require_source('ConduitAPI_differential_close_Method.php');

src/applications/conduit/method/differential/creatediff/ConduitAPI_differential_creatediff_Method.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ protected function execute(ConduitAPIRequest $request) {
7676
$parent_rev = id(new DifferentialRevision())->load($parent_id);
7777
if ($parent_rev) {
7878
if ($parent_rev->getStatus() !=
79-
ArcanistDifferentialRevisionStatus::COMMITTED) {
79+
ArcanistDifferentialRevisionStatus::CLOSED) {
8080
$diff->setParentRevisionID($parent_id);
8181
}
8282
}

src/applications/conduit/method/differential/markcommitted/ConduitAPI_differential_markcommitted_Method.php

+12-11
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,21 @@
1818

1919
/**
2020
* @group conduit
21+
* @deprecated
2122
*/
2223
final class ConduitAPI_differential_markcommitted_Method
2324
extends ConduitAPIMethod {
2425

26+
public function getMethodStatus() {
27+
return self::METHOD_STATUS_DEPRECATED;
28+
}
29+
30+
public function getMethodStatusDescription() {
31+
return "Replaced by 'differential.close'.";
32+
}
33+
2534
public function getMethodDescription() {
26-
return "Mark Differential revisions as committed.";
35+
return "Mark a revision closed.";
2736
}
2837

2938
public function defineParamTypes() {
@@ -50,13 +59,7 @@ protected function execute(ConduitAPIRequest $request) {
5059
throw new ConduitException('ERR_NOT_FOUND');
5160
}
5261

53-
if ($revision->getStatus() ==
54-
ArcanistDifferentialRevisionStatus::COMMITTED) {
55-
// This can occur if someone runs 'mark-committed' and hits a race, or
56-
// they have a remote hook installed but don't have the
57-
// 'remote_hook_installed' flag set, or similar. In any case, just treat
58-
// it as a no-op rather than adding another "X committed this revision"
59-
// message to the revision comments.
62+
if ($revision->getStatus() == ArcanistDifferentialRevisionStatus::CLOSED) {
6063
return;
6164
}
6265

@@ -65,10 +68,8 @@ protected function execute(ConduitAPIRequest $request) {
6568
$editor = new DifferentialCommentEditor(
6669
$revision,
6770
$request->getUser()->getPHID(),
68-
DifferentialAction::ACTION_COMMIT);
71+
DifferentialAction::ACTION_CLOSE);
6972
$editor->save();
70-
71-
return;
7273
}
7374

7475
}

src/applications/conduit/method/differential/query/ConduitAPI_differential_query_Method.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public function defineParamTypes() {
3434
DifferentialRevisionQuery::STATUS_ANY,
3535
DifferentialRevisionQuery::STATUS_OPEN,
3636
DifferentialRevisionQuery::STATUS_ACCEPTED,
37-
DifferentialRevisionQuery::STATUS_COMMITTED,
37+
DifferentialRevisionQuery::STATUS_CLOSED,
3838
);
3939
$status_types = implode(', ', $status_types);
4040

src/applications/conduit/method/differential/updaterevision/ConduitAPI_differential_updaterevision_Method.php

+3-4
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public function defineErrorTypes() {
4444
'ERR_BAD_DIFF' => 'Bad diff ID.',
4545
'ERR_BAD_REVISION' => 'Bad revision ID.',
4646
'ERR_WRONG_USER' => 'You are not the author of this revision.',
47-
'ERR_COMMITTED' => 'This revision has already been committed.',
47+
'ERR_CLOSED' => 'This revision has already been closed.',
4848
);
4949
}
5050

@@ -63,9 +63,8 @@ protected function execute(ConduitAPIRequest $request) {
6363
throw new ConduitException('ERR_WRONG_USER');
6464
}
6565

66-
if ($revision->getStatus() ==
67-
ArcanistDifferentialRevisionStatus::COMMITTED) {
68-
throw new ConduitException('ERR_COMMITTED');
66+
if ($revision->getStatus() == ArcanistDifferentialRevisionStatus::CLOSED) {
67+
throw new ConduitException('ERR_CLOSED');
6968
}
7069

7170
$content_source = PhabricatorContentSource::newForSource(

src/applications/differential/constants/action/DifferentialAction.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
final class DifferentialAction {
2020

21-
const ACTION_COMMIT = 'commit';
21+
const ACTION_CLOSE = 'commit';
2222
const ACTION_COMMENT = 'none';
2323
const ACTION_ACCEPT = 'accept';
2424
const ACTION_REJECT = 'reject';
@@ -42,7 +42,7 @@ public static function getActionPastTenseVerb($action) {
4242
self::ACTION_REJECT => 'requested changes to',
4343
self::ACTION_RETHINK => 'planned changes to',
4444
self::ACTION_ABANDON => 'abandoned',
45-
self::ACTION_COMMIT => 'committed',
45+
self::ACTION_CLOSE => 'closed',
4646
self::ACTION_REQUEST => 'requested a review of',
4747
self::ACTION_RECLAIM => 'reclaimed',
4848
self::ACTION_UPDATE => 'updated',
@@ -74,7 +74,7 @@ public static function getActionVerb($action) {
7474
self::ACTION_RESIGN => 'Resign as Reviewer',
7575
self::ACTION_ADDREVIEWERS => 'Add Reviewers',
7676
self::ACTION_ADDCCS => 'Add CCs',
77-
self::ACTION_COMMIT => 'Mark Committed',
77+
self::ACTION_CLOSE => 'Close Revision',
7878
self::ACTION_CLAIM => 'Commandeer Revision',
7979
);
8080

src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ private function renderControl(
379379
array(
380380
'all' => 'All',
381381
'open' => 'Open',
382-
'committed' => 'Committed',
382+
'closed' => 'Closed',
383383
'abandoned' => 'Abandoned',
384384
));
385385
case 'order':
@@ -406,8 +406,8 @@ private function applyControlToQuery($control, $query, array $params) {
406406
case 'status':
407407
if ($params['status'] == 'open') {
408408
$query->withStatus(DifferentialRevisionQuery::STATUS_OPEN);
409-
} else if ($params['status'] == 'committed') {
410-
$query->withStatus(DifferentialRevisionQuery::STATUS_COMMITTED);
409+
} else if ($params['status'] == 'closed') {
410+
$query->withStatus(DifferentialRevisionQuery::STATUS_CLOSED);
411411
} else if ($params['status'] == 'abandoned') {
412412
$query->withStatus(DifferentialRevisionQuery::STATUS_ABANDONED);
413413
}

src/applications/differential/controller/revisionstats/DifferentialRevisionStatsController.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public function processRequest() {
101101
$side_nav->setBaseURI(id(new PhutilURI('/differential/stats/'))
102102
->alter('phid', $params['phid']));
103103
foreach (array(
104-
DifferentialAction::ACTION_COMMIT,
104+
DifferentialAction::ACTION_CLOSE,
105105
DifferentialAction::ACTION_ACCEPT,
106106
DifferentialAction::ACTION_REJECT,
107107
DifferentialAction::ACTION_UPDATE,
@@ -112,7 +112,7 @@ public function processRequest() {
112112
}
113113
$this->filter =
114114
$side_nav->selectFilter($this->filter,
115-
DifferentialAction::ACTION_COMMIT);
115+
DifferentialAction::ACTION_CLOSE);
116116

117117
$panels = array();
118118
$handles = id(new PhabricatorObjectHandleData(array($params['phid'])))

src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -499,9 +499,9 @@ private function getRevisionCommentActions(DifferentialRevision $revision) {
499499
$actions[DifferentialAction::ACTION_ABANDON] = true;
500500
$actions[DifferentialAction::ACTION_REQUEST] = true;
501501
$actions[DifferentialAction::ACTION_RETHINK] = true;
502-
$actions[DifferentialAction::ACTION_COMMIT] = true;
502+
$actions[DifferentialAction::ACTION_CLOSE] = true;
503503
break;
504-
case ArcanistDifferentialRevisionStatus::COMMITTED:
504+
case ArcanistDifferentialRevisionStatus::CLOSED:
505505
break;
506506
case ArcanistDifferentialRevisionStatus::ABANDONED:
507507
$actions[DifferentialAction::ACTION_RECLAIM] = true;
@@ -523,11 +523,11 @@ private function getRevisionCommentActions(DifferentialRevision $revision) {
523523
$actions[DifferentialAction::ACTION_RESIGN] =
524524
$viewer_is_reviewer && !$viewer_did_accept;
525525
break;
526-
case ArcanistDifferentialRevisionStatus::COMMITTED:
526+
case ArcanistDifferentialRevisionStatus::CLOSED:
527527
case ArcanistDifferentialRevisionStatus::ABANDONED:
528528
break;
529529
}
530-
if ($status != ArcanistDifferentialRevisionStatus::COMMITTED) {
530+
if ($status != ArcanistDifferentialRevisionStatus::CLOSED) {
531531
$actions[DifferentialAction::ACTION_CLAIM] = true;
532532
}
533533
}

0 commit comments

Comments
 (0)