mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-10 08:52:39 +01:00
Remove client-side commit message validation from Arcanist
Summary: Commit templates are fully configurable on the server now, so we should be doing validation there, since an install can add or remove fields and change validation rules. Remove these outdated client validations, as per comment. Also update the API call to use the 'errors' field, which allows us to show the user all the parse errors at once. See: https://secure.phabricator.com/diffusion/P/browse/origin:master/src/applications/conduit/method/differential/parsecommitmessage/ConduitAPI_differential_parsecommitmessage_Method.php;cfaab709df37739b$75 Test Plan: Ran "arc diff" on a change with multiple errors: ```$ arc diff --conduit-uri=http://local.aphront.com/ Usage Exception: Commit message is not properly formatted: Error parsing field 'Reviewers': Commit message references nonexistent users: jjjderp. Error parsing field 'CC': Commit message references nonexistent users and mailing lists: jjjderp. You should use the standard git commit template to provide a commit message. If you only want to create a diff (not a revision), use --preview to ignore commit messages.``` Reviewers: btrahan, jungejason Reviewed By: btrahan CC: aran, btrahan Differential Revision: 1154
This commit is contained in:
parent
d81d97f9c6
commit
79da3a6ff9
4 changed files with 19 additions and 28 deletions
|
@ -66,9 +66,9 @@ class ArcanistDifferentialCommitMessage {
|
|||
'corpus' => $this->rawCorpus,
|
||||
));
|
||||
$result = $result->resolve();
|
||||
if (!empty($result['error'])) {
|
||||
if (!empty($result['errors'])) {
|
||||
throw new ArcanistDifferentialCommitMessageParserException(
|
||||
$result['error']);
|
||||
$result['errors']);
|
||||
}
|
||||
$this->fields = $result['fields'];
|
||||
}
|
||||
|
|
|
@ -23,4 +23,15 @@
|
|||
*/
|
||||
class ArcanistDifferentialCommitMessageParserException extends Exception {
|
||||
|
||||
private $parserErrors;
|
||||
|
||||
public function __construct(array $errors) {
|
||||
$this->parserErrors = $errors;
|
||||
parent::__construct(head($errors));
|
||||
}
|
||||
|
||||
public function getParserErrors() {
|
||||
return $this->parserErrors;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -6,6 +6,8 @@
|
|||
|
||||
|
||||
|
||||
phutil_require_module('phutil', 'utils');
|
||||
|
||||
|
||||
phutil_require_source('ArcanistDifferentialCommitMessage.php');
|
||||
phutil_require_source('ArcanistDifferentialCommitMessageParserException.php');
|
||||
|
|
|
@ -972,32 +972,11 @@ EOTEXT
|
|||
|
||||
$parsed[$key] = $message;
|
||||
} catch (ArcanistDifferentialCommitMessageParserException $ex) {
|
||||
$problems[$key][] = $ex;
|
||||
foreach ($ex->getParserErrors() as $problem) {
|
||||
$problems[$key][] = $problem;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
// TODO: Move this all behind Conduit.
|
||||
if (!$message->getRevisionID()) {
|
||||
if ($message->getFieldValue('reviewedByPHIDs')) {
|
||||
$problems[$key][] = new ArcanistUsageException(
|
||||
"When creating or updating a revision, use the 'Reviewers:' ".
|
||||
"field to specify reviewers, not 'Reviewed By:'. After the ".
|
||||
"revision is accepted, run 'arc amend' to update the commit ".
|
||||
"message.");
|
||||
}
|
||||
|
||||
if (!$message->getFieldValue('title')) {
|
||||
$problems[$key][] = new ArcanistUsageException(
|
||||
"Commit message has no title. You must provide a title for this ".
|
||||
"revision.");
|
||||
}
|
||||
|
||||
if (!$message->getFieldValue('testPlan')) {
|
||||
$problems[$key][] = new ArcanistUsageException(
|
||||
"Commit message has no 'Test Plan:'. You must provide a test ".
|
||||
"plan.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$blessed = null;
|
||||
|
@ -1020,8 +999,7 @@ EOTEXT
|
|||
}
|
||||
|
||||
if ($revision_id === -1) {
|
||||
$all_problems = call_user_func_array('array_merge', $problems);
|
||||
$desc = implode("\n", mpull($all_problems, 'getMessage'));
|
||||
$desc = implode("\n", array_mergev($problems));
|
||||
if (count($problems) > 1) {
|
||||
throw new ArcanistUsageException(
|
||||
"All changes between the specified commits have template parsing ".
|
||||
|
|
Loading…
Reference in a new issue