Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

Commit

Permalink
checkcommits: allow subsystem to overide need for "Fixes #XXX"
Browse files Browse the repository at this point in the history
Add a new `--ignore-fixes-for-subsystem=` option that if specified will
disable the effect of `--need-fixes` for those branches containing atleast one
commit with the specified subsystem.

The most useful scenario for this option being to allow a release PR
(subsystem "`release: ...`") to not require an issue to be raised for it.

Fixes #820.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
  • Loading branch information
jodh-intel committed Jan 19, 2018
1 parent c3ea500 commit b383433
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 5 deletions.
29 changes: 28 additions & 1 deletion cmd/checkcommits/checkcommits.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type CommitConfig struct {
SobString string
FixesString string

// Ignore NeedFixes if the subsystem matches this value.
IgnoreFixesSubsystem string

FixesPattern *regexp.Regexp
SobPattern *regexp.Regexp
}
Expand Down Expand Up @@ -411,6 +414,23 @@ func checkCommitsDetails(config *CommitConfig, commits []Commit) (err error) {
results = append(results, commit)
}

ignoreFixesSubsystem := false

if config.IgnoreFixesSubsystem != "" {
for _, commit := range results {
if strings.HasPrefix(commit.subsystem, config.IgnoreFixesSubsystem) {
ignoreFixesSubsystem = true
}
}
}

if ignoreFixesSubsystem {
// Fixes isn't required for the entire commit range
// due to the specified subsystem being found in one of the
// commits.
config.NeedFixes = false
}

if config.NeedFixes && !config.FoundFixes {
return fmt.Errorf("No %q found", config.FixesString)
}
Expand Down Expand Up @@ -535,14 +555,15 @@ func runCommand(args []string) (stdout []string, err error) {
}

// NewCommitConfig creates a new CommitConfig object.
func NewCommitConfig(needFixes, needSignOffs bool, fixesPrefix, signoffPrefix string, bodyLength, subjectLength int) *CommitConfig {
func NewCommitConfig(needFixes, needSignOffs bool, fixesPrefix, signoffPrefix, ignoreFixesForSubsystem string, bodyLength, subjectLength int) *CommitConfig {
config := &CommitConfig{
NeedSOBS: needSignOffs,
NeedFixes: needFixes,
MaxBodyLineLength: bodyLength,
MaxSubjectLineLength: subjectLength,
SobString: defaultSobString,
FixesString: defaultFixesString,
IgnoreFixesSubsystem: ignoreFixesForSubsystem,
}

if config.MaxBodyLineLength == 0 {
Expand Down Expand Up @@ -687,6 +708,7 @@ func checkCommitsAction(c *cli.Context) error {
c.Bool("need-sign-offs"),
c.String("fixes-prefix"),
c.String("sign-off-prefix"),
c.String("ignore-fixes-for-subsystem"),
int(c.Uint("body-length")),
int(c.Uint("subject-length")))

Expand Down Expand Up @@ -735,6 +757,11 @@ func main() {
Destination: &debug,
},

cli.StringFlag{
Name: "ignore-fixes-for-subsystem",
Usage: fmt.Sprintf("Don't requires a Fixes comment if the subsystem matches the specified string"),
},

cli.StringFlag{
Name: "fixes-prefix",
Usage: fmt.Sprintf("Fixes `prefix` used as an alternative to %q", defaultFixesString),
Expand Down
19 changes: 15 additions & 4 deletions cmd/checkcommits/checkcommits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ func createCommitConfig() (config *CommitConfig) {
return NewCommitConfig(true, true,
testFixesString,
"Signed-off-by",
"",
defaultMaxBodyLineLength,
defaultMaxSubjectLineLength)
}
Expand Down Expand Up @@ -258,10 +259,13 @@ func TestCheckCommits(t *testing.T) {
func TestCheckCommitsDetails(t *testing.T) {
assert := assert.New(t)

makeConfig := func() *CommitConfig {
ignoreSubsystem := "release"

makeConfigWithIgnoreSubsys := func(ignoreFixesSubsystem string) *CommitConfig {
return NewCommitConfig(true, true,
testFixesString,
"Signed-off-by",
ignoreFixesSubsystem,
defaultMaxBodyLineLength,
defaultMaxSubjectLineLength)
}
Expand Down Expand Up @@ -292,10 +296,17 @@ func TestCheckCommitsDetails(t *testing.T) {

data := []testData{
// A "normal" commit
{makeConfig(), makeCommits("foo", "Fixes #123"), false},
{makeConfigWithIgnoreSubsys(""), makeCommits("foo", "Fixes #123"), false},

// Releases don't require a Fixes comment
{makeConfigWithIgnoreSubsys(ignoreSubsystem), makeCommits(ignoreSubsystem, "foo"), false},

// Valid since there is no instance of ignoreSubsystem and the
// commits are "well-formed".
{makeConfigWithIgnoreSubsys(ignoreSubsystem), makeCommits("foo", "Fixes #123"), false},

// Fails as no "Fixes #XXX"
{makeConfig(), makeCommits("foo", "foo"), true},
{makeConfigWithIgnoreSubsys(""), makeCommits(ignoreSubsystem, "foo"), true},
}

for _, d := range data {
Expand All @@ -315,7 +326,7 @@ func TestCheckCommit(t *testing.T) {
err := checkCommit(nil, nil)
assert.Error(err, "expected error when no config specified")

config := NewCommitConfig(true, true, "", "", 0, 0)
config := NewCommitConfig(true, true, "", "", "", 0, 0)
err = checkCommit(config, nil)
assert.Error(err, "expected error when no commit specified")

Expand Down

0 comments on commit b383433

Please sign in to comment.