From 2d9d2f1aede1e2e3b38679613bb32ab80f5d547b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 May 2012 07:35:56 -0700 Subject: [PATCH] Allow "Differential Revision ID" field appear on create/edit messages Summary: - In practice, 'edit' has two modes, 'create' and 'edit'. These seem like they should map to "create a revision" and "update a revision", but they are completely different. - We use the "create" mode: - When creating a message from the working copy. - When creating a message from a file. - When creating a message from a commit. - When creating a message from a user template. - When creating a message from an "--edit"! - We use the "edit" mode: - ONLY when updating a revision with `arc diff --verbatim`. - The only difference is in which fields may be overwritten. Under "create", all fields may be overwritten. Under "edit", only safe fields may be overwritten. - The "Differential Revision" field currently does not render in either edit mode. This is wrong. Even though it can not be updated in the "edit" mode, it should still render in both modes. This is the only material change this revision makes. - Without this change, when we "create" a new message from a working copy and the working copy has a "Differential Revision" field, we incorrectly discard it. - The only field which does not render on edit modes now is "Reviewed by" (not "Reviewers"), which is correct, since we do not read the value. Test Plan: Ran "arc diff" to create/update revisions. Ran "arc diff --verbatim" to create/update revisions with implicit edits (with D2411). Ran "arc diff --edit" to update revisions with explicit edits. Reviewers: jungejason, btrahan Reviewed by: jungejason CC: vrana, aran Differential Revision: https://secure.phabricator.com/D2412 --- .../conduit/connect/ConduitAPI_conduit_connect_Method.php | 2 +- .../field/specification/base/DifferentialFieldSpecification.php | 2 ++ .../revisionid/DifferentialRevisionIDFieldSpecification.php | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php b/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php index 3d2a7ff9a4..dfa88b2273 100644 --- a/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php +++ b/src/applications/conduit/method/conduit/connect/ConduitAPI_conduit_connect_Method.php @@ -86,7 +86,7 @@ final class ConduitAPI_conduit_connect_Method extends ConduitAPIMethod { switch ($client) { case 'arc': - $server_version = 3; + $server_version = 4; switch ($client_version) { case $server_version: break; diff --git a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php index 7237c9b1e3..9d47c9c57d 100644 --- a/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php +++ b/src/applications/differential/field/specification/base/DifferentialFieldSpecification.php @@ -431,6 +431,8 @@ abstract class DifferentialFieldSpecification { * message, return true. If the authoritative value should always be used, * return false. By default, fields can not be overwritten. * + * arc will only attempt to overwrite field values if run with "--verbatim". + * * @return bool True to indicate the field is save to overwrite. * @task commit */ diff --git a/src/applications/differential/field/specification/revisionid/DifferentialRevisionIDFieldSpecification.php b/src/applications/differential/field/specification/revisionid/DifferentialRevisionIDFieldSpecification.php index 9d451b1252..96e8c956ce 100644 --- a/src/applications/differential/field/specification/revisionid/DifferentialRevisionIDFieldSpecification.php +++ b/src/applications/differential/field/specification/revisionid/DifferentialRevisionIDFieldSpecification.php @@ -47,7 +47,7 @@ final class DifferentialRevisionIDFieldSpecification } public function renderValueForCommitMessage($is_edit) { - if ($is_edit || !$this->id) { + if (!$this->id) { return null; } return PhabricatorEnv::getProductionURI('/D'.$this->id);