Homec4science

Validate all fields in differential.parsecommitmessage

Authored by epriestley <git@epriestley.com> on Jan 12 2012, 04:55.

Description

Validate all fields in differential.parsecommitmessage

Summary:

  • We currently run parseValueFromCommitMessage() on all fields present in

the message, but not validateField().

  • This detects value errors (e.g., an invalid reviewer) but not higher-level

errors (e.g., a missing field).

  • This can break the stacked-commits Git mutable history workflow by

recognizing too many commit messages as valid ("multiple valid commit messages,
this is ambiguous").

  • This also gives you some errors ("Missing test plan") too late in "arc diff

--create" (after the diff has been built).

Test Plan:

  • Grepped for validateField() calls, removed a couple of calls that had the

same implementation as the base class.

  • Grepped for other calls to this to make sure I'm not stumbling into

unintended side effects, but it only runs from the diff workflow.

  • Ran "arc diff --create" with an invalid test plan, got a good error early in

the process.

  • Ran "arc diff master" with stacked local commits, got a correct selection of

the intended message.

Reviewers: cpiro, btrahan, jungejason

Reviewed By: cpiro

CC: aran, cpiro

Differential Revision: https://secure.phabricator.com/D1373

Details

Committed
epriestley <git@epriestley.com>Jan 13 2012, 00:20
Pushed
aubortJan 31 2017, 17:16
Parents
rPH02fb5fea89e6: Allow configuration of a minimum password length, unify password reset…
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPHd84c0a5be59c: Validate all fields in differential.parsecommitmessage (authored by epriestley <git@epriestley.com>).Jan 13 2012, 00:20