mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-23 15:22:41 +01:00
Parse VCS revert langauge in commit messages
Summary: Ref T1751. When commit messages include language like "reverts X", parse it. This change doesn't do anything with the commits yet. I attempted to cover all "natural" VCS messages and all reasonable human variations of these messages. Test Plan: Added unit tests. Added `var_dump()` and used `scripts/repository/reparse.php --message X` to reparse some commit messages, with expected results. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1751 Differential Revision: https://secure.phabricator.com/D5840
This commit is contained in:
parent
b51117790e
commit
2b2641192a
3 changed files with 86 additions and 1 deletions
|
@ -334,6 +334,7 @@ phutil_register_library_map(array(
|
||||||
'DifferentialFieldSpecificationIncompleteException' => 'applications/differential/field/exception/DifferentialFieldSpecificationIncompleteException.php',
|
'DifferentialFieldSpecificationIncompleteException' => 'applications/differential/field/exception/DifferentialFieldSpecificationIncompleteException.php',
|
||||||
'DifferentialFieldValidationException' => 'applications/differential/field/exception/DifferentialFieldValidationException.php',
|
'DifferentialFieldValidationException' => 'applications/differential/field/exception/DifferentialFieldValidationException.php',
|
||||||
'DifferentialFreeformFieldSpecification' => 'applications/differential/field/specification/DifferentialFreeformFieldSpecification.php',
|
'DifferentialFreeformFieldSpecification' => 'applications/differential/field/specification/DifferentialFreeformFieldSpecification.php',
|
||||||
|
'DifferentialFreeformFieldTestCase' => 'applications/differential/field/specification/__tests__/DifferentialFreeformFieldTestCase.php',
|
||||||
'DifferentialGitSVNIDFieldSpecification' => 'applications/differential/field/specification/DifferentialGitSVNIDFieldSpecification.php',
|
'DifferentialGitSVNIDFieldSpecification' => 'applications/differential/field/specification/DifferentialGitSVNIDFieldSpecification.php',
|
||||||
'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/DifferentialHostFieldSpecification.php',
|
'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/DifferentialHostFieldSpecification.php',
|
||||||
'DifferentialHovercardEventListener' => 'applications/differential/events/DifferentialHovercardEventListener.php',
|
'DifferentialHovercardEventListener' => 'applications/differential/events/DifferentialHovercardEventListener.php',
|
||||||
|
@ -1221,6 +1222,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorPhabricatorOAuthConfigOptions' => 'applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php',
|
'PhabricatorPhabricatorOAuthConfigOptions' => 'applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php',
|
||||||
'PhabricatorPhameConfigOptions' => 'applications/phame/config/PhabricatorPhameConfigOptions.php',
|
'PhabricatorPhameConfigOptions' => 'applications/phame/config/PhabricatorPhameConfigOptions.php',
|
||||||
'PhabricatorPholioConfigOptions' => 'applications/pholio/config/PhabricatorPholioConfigOptions.php',
|
'PhabricatorPholioConfigOptions' => 'applications/pholio/config/PhabricatorPholioConfigOptions.php',
|
||||||
|
'PhabricatorPholioMockTestDataGenerator' => 'applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php',
|
||||||
'PhabricatorPhortuneConfigOptions' => 'applications/phortune/option/PhabricatorPhortuneConfigOptions.php',
|
'PhabricatorPhortuneConfigOptions' => 'applications/phortune/option/PhabricatorPhortuneConfigOptions.php',
|
||||||
'PhabricatorPhrequentConfigOptions' => 'applications/phrequent/config/PhabricatorPhrequentConfigOptions.php',
|
'PhabricatorPhrequentConfigOptions' => 'applications/phrequent/config/PhabricatorPhrequentConfigOptions.php',
|
||||||
'PhabricatorPhrictionConfigOptions' => 'applications/phriction/config/PhabricatorPhrictionConfigOptions.php',
|
'PhabricatorPhrictionConfigOptions' => 'applications/phriction/config/PhabricatorPhrictionConfigOptions.php',
|
||||||
|
@ -2099,6 +2101,7 @@ phutil_register_library_map(array(
|
||||||
'DifferentialFieldSpecificationIncompleteException' => 'Exception',
|
'DifferentialFieldSpecificationIncompleteException' => 'Exception',
|
||||||
'DifferentialFieldValidationException' => 'Exception',
|
'DifferentialFieldValidationException' => 'Exception',
|
||||||
'DifferentialFreeformFieldSpecification' => 'DifferentialFieldSpecification',
|
'DifferentialFreeformFieldSpecification' => 'DifferentialFieldSpecification',
|
||||||
|
'DifferentialFreeformFieldTestCase' => 'PhabricatorTestCase',
|
||||||
'DifferentialGitSVNIDFieldSpecification' => 'DifferentialFieldSpecification',
|
'DifferentialGitSVNIDFieldSpecification' => 'DifferentialFieldSpecification',
|
||||||
'DifferentialHostFieldSpecification' => 'DifferentialFieldSpecification',
|
'DifferentialHostFieldSpecification' => 'DifferentialFieldSpecification',
|
||||||
'DifferentialHovercardEventListener' => 'PhutilEventListener',
|
'DifferentialHovercardEventListener' => 'PhutilEventListener',
|
||||||
|
@ -2935,6 +2938,7 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorPhabricatorOAuthConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
'PhabricatorPhabricatorOAuthConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
||||||
'PhabricatorPhameConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
'PhabricatorPhameConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
||||||
'PhabricatorPholioConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
'PhabricatorPholioConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
||||||
|
'PhabricatorPholioMockTestDataGenerator' => 'PhabricatorTestDataGenerator',
|
||||||
'PhabricatorPhortuneConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
'PhabricatorPhortuneConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
||||||
'PhabricatorPhrequentConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
'PhabricatorPhrequentConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
||||||
'PhabricatorPhrictionConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
'PhabricatorPhrictionConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
||||||
|
|
|
@ -98,6 +98,53 @@ abstract class DifferentialFreeformFieldSpecification
|
||||||
return $dependents;
|
return $dependents;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static function findRevertedCommits($message) {
|
||||||
|
$reverts = array();
|
||||||
|
$matches = null;
|
||||||
|
|
||||||
|
// NOTE: Git language is "This reverts commit X."
|
||||||
|
// NOTE: Mercurial language is "Backed out changeset Y".
|
||||||
|
|
||||||
|
$prefixes = array(
|
||||||
|
'revert' => true,
|
||||||
|
'reverts' => true,
|
||||||
|
'back\s*out' => true,
|
||||||
|
'backs\s*out' => true,
|
||||||
|
'backed\s*out' => true,
|
||||||
|
'undo' => true,
|
||||||
|
'undoes' => true,
|
||||||
|
);
|
||||||
|
|
||||||
|
$optional = array(
|
||||||
|
'commit' => true,
|
||||||
|
'changeset' => true,
|
||||||
|
'rev' => true,
|
||||||
|
'revision' => true,
|
||||||
|
'change' => true,
|
||||||
|
'diff' => true,
|
||||||
|
);
|
||||||
|
|
||||||
|
$pre_re = implode('|', array_keys($prefixes));
|
||||||
|
$opt_re = implode('|', array_keys($optional));
|
||||||
|
|
||||||
|
$matches = null;
|
||||||
|
preg_match_all(
|
||||||
|
'/\b(?i:'.$pre_re.')(?:\s+(?i:'.$opt_re.'))?([rA-Z0-9a-f,\s]+)\b/',
|
||||||
|
$message,
|
||||||
|
$matches);
|
||||||
|
|
||||||
|
$result = array();
|
||||||
|
foreach ($matches[1] as $commits) {
|
||||||
|
$commits = preg_split('/[,\s]+/', $commits);
|
||||||
|
$commits = array_filter($commits);
|
||||||
|
foreach ($commits as $commit) {
|
||||||
|
$result[$commit] = $commit;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $result;
|
||||||
|
}
|
||||||
|
|
||||||
public function didWriteRevision(DifferentialRevisionEditor $editor) {
|
public function didWriteRevision(DifferentialRevisionEditor $editor) {
|
||||||
$message = $this->renderValueForCommitMessage(false);
|
$message = $this->renderValueForCommitMessage(false);
|
||||||
|
|
||||||
|
@ -151,6 +198,10 @@ abstract class DifferentialFreeformFieldSpecification
|
||||||
PhabricatorRepositoryCommit $commit,
|
PhabricatorRepositoryCommit $commit,
|
||||||
PhabricatorRepositoryCommitData $data) {
|
PhabricatorRepositoryCommitData $data) {
|
||||||
|
|
||||||
|
$message = $this->renderValueForCommitMessage($is_edit = false);
|
||||||
|
|
||||||
|
$commits = self::findRevertedCommits($message);
|
||||||
|
|
||||||
$user = id(new PhabricatorUser())->loadOneWhere(
|
$user = id(new PhabricatorUser())->loadOneWhere(
|
||||||
'phid = %s',
|
'phid = %s',
|
||||||
$data->getCommitDetail('authorPHID'));
|
$data->getCommitDetail('authorPHID'));
|
||||||
|
@ -158,7 +209,6 @@ abstract class DifferentialFreeformFieldSpecification
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
$message = $this->renderValueForCommitMessage($is_edit = false);
|
|
||||||
$tasks_statuses = $this->findMentionedTasks($message);
|
$tasks_statuses = $this->findMentionedTasks($message);
|
||||||
if (!$tasks_statuses) {
|
if (!$tasks_statuses) {
|
||||||
return;
|
return;
|
||||||
|
|
|
@ -0,0 +1,31 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class DifferentialFreeformFieldTestCase extends PhabricatorTestCase {
|
||||||
|
|
||||||
|
public function testRevertedCommitParser() {
|
||||||
|
$map = array(
|
||||||
|
"Reverts 123" => array('123'),
|
||||||
|
"Reverts r123" => array('r123'),
|
||||||
|
"Reverts ac382f2" => array('ac382f2'),
|
||||||
|
"Reverts r22, r23" => array('r22', 'r23'),
|
||||||
|
"Reverts D99" => array('D99'),
|
||||||
|
"Backs out commit\n99\n100" => array('99', '100'),
|
||||||
|
"undo change f9f9f8f8" => array('f9f9f8f8'),
|
||||||
|
"Backedout Changeset rX1234" => array('rX1234'),
|
||||||
|
"This doesn't revert anything" => array(),
|
||||||
|
'nonrevert of r11' => array(),
|
||||||
|
"fixed a bug" => array(),
|
||||||
|
);
|
||||||
|
|
||||||
|
foreach ($map as $input => $expect) {
|
||||||
|
$actual = array_values(
|
||||||
|
DifferentialFreeformFieldSpecification::findRevertedCommits($input));
|
||||||
|
|
||||||
|
$this->assertEqual(
|
||||||
|
$expect,
|
||||||
|
$actual,
|
||||||
|
"Reverted commits in: {$input}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
Loading…
Reference in a new issue