From 0b1d6a3f6ec6bb571ff69f7203d15f9f9d2aaa03 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 11 Aug 2017 15:55:48 -0700 Subject: [PATCH] Convert straggling Herald rules to modern revision status constants Summary: Ref T2543. These are the last `ArcanistDifferentialRevisionStatus` callsites. This removes the very old legacy `precommitRevisionStatus` field, which has no other readers. This was obsoleted by the `CLOSED_FROM_ACCEPTED` stuff, but retained for compatibility. Test Plan: - Poked these with the test console, although they're a little tricky to be sure about. - Grepped for `ArcanistDifferentialRevisionStatus`, no more hits. Reviewers: chad Reviewed By: chad Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18416 --- ...usionCommitRevisionAcceptedHeraldField.php | 33 ++++--------------- ...mmitContentRevisionAcceptedHeraldField.php | 18 +++++----- ...torRepositoryCommitMessageParserWorker.php | 5 --- 3 files changed, 14 insertions(+), 42 deletions(-) diff --git a/src/applications/diffusion/herald/DiffusionCommitRevisionAcceptedHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitRevisionAcceptedHeraldField.php index 810b58f2a1..a125b580eb 100644 --- a/src/applications/diffusion/herald/DiffusionCommitRevisionAcceptedHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionCommitRevisionAcceptedHeraldField.php @@ -19,36 +19,15 @@ final class DiffusionCommitRevisionAcceptedHeraldField return null; } - $status = $revision->getStatus(); - - switch ($status) { - case ArcanistDifferentialRevisionStatus::ACCEPTED: - return $revision->getPHID(); - case ArcanistDifferentialRevisionStatus::CLOSED: - if ($revision->hasRevisionProperty( - DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED)) { - - if ($revision->getProperty( - DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED)) { - return $revision->getPHID(); - } else { - return null; - } - } else { - // continue on to old-style precommitRevisionStatus - break; - } - default: - return null; + if ($revision->isAccepted()) { + return $revision->getPHID(); } - $data = $object->getCommitData(); - $status = $data->getCommitDetail('precommitRevisionStatus'); - - switch ($status) { - case ArcanistDifferentialRevisionStatus::ACCEPTED: - case ArcanistDifferentialRevisionStatus::CLOSED: + $was_accepted = DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED; + if ($revision->isPublished()) { + if ($revision->getProperty($was_accepted)) { return $revision->getPHID(); + } } return null; diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptedHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptedHeraldField.php index e19878e627..63dac6baa6 100644 --- a/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptedHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptedHeraldField.php @@ -15,21 +15,19 @@ final class DiffusionPreCommitContentRevisionAcceptedHeraldField public function getHeraldFieldValue($object) { $revision = $this->getAdapter()->getRevision(); - if (!$revision) { return null; } - switch ($revision->getStatus()) { - case ArcanistDifferentialRevisionStatus::ACCEPTED: - return $revision->getPHID(); - case ArcanistDifferentialRevisionStatus::CLOSED: - if ($revision->getProperty( - DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED)) { + if ($revision->isAccepted()) { + return $revision->getPHID(); + } - return $revision->getPHID(); - } - break; + $was_accepted = DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED; + if ($revision->isPublished()) { + if ($revision->getProperty($was_accepted)) { + return $revision->getPHID(); + } } return null; diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 0d0ff28d95..fe8abd5925 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -186,11 +186,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $revision = $revision_query->executeOne(); if ($revision) { - if (!$data->getCommitDetail('precommitRevisionStatus')) { - $data->setCommitDetail( - 'precommitRevisionStatus', - $revision->getStatus()); - } $commit_drev = DiffusionCommitHasRevisionEdgeType::EDGECONST; id(new PhabricatorEdgeEditor()) ->addEdge($commit->getPHID(), $commit_drev, $revision->getPHID())