From b383433b4f17accfc8dfeab1c8a3d8890888f736 Mon Sep 17 00:00:00 2001
From: "James O. D. Hunt" <james.o.hunt@intel.com>
Date: Fri, 19 Jan 2018 12:20:44 +0000
Subject: [PATCH] checkcommits: allow subsystem to overide need for "Fixes
 #XXX"

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>
---
 cmd/checkcommits/checkcommits.go      | 29 ++++++++++++++++++++++++++-
 cmd/checkcommits/checkcommits_test.go | 19 ++++++++++++++----
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/cmd/checkcommits/checkcommits.go b/cmd/checkcommits/checkcommits.go
index 35f731aad..9c997cfad 100644
--- a/cmd/checkcommits/checkcommits.go
+++ b/cmd/checkcommits/checkcommits.go
@@ -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
 }
@@ -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)
 	}
@@ -535,7 +555,7 @@ 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,
@@ -543,6 +563,7 @@ func NewCommitConfig(needFixes, needSignOffs bool, fixesPrefix, signoffPrefix st
 		MaxSubjectLineLength: subjectLength,
 		SobString:            defaultSobString,
 		FixesString:          defaultFixesString,
+		IgnoreFixesSubsystem: ignoreFixesForSubsystem,
 	}
 
 	if config.MaxBodyLineLength == 0 {
@@ -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")))
 
@@ -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),
diff --git a/cmd/checkcommits/checkcommits_test.go b/cmd/checkcommits/checkcommits_test.go
index 268508837..0c39f7c82 100644
--- a/cmd/checkcommits/checkcommits_test.go
+++ b/cmd/checkcommits/checkcommits_test.go
@@ -112,6 +112,7 @@ func createCommitConfig() (config *CommitConfig) {
 	return NewCommitConfig(true, true,
 		testFixesString,
 		"Signed-off-by",
+		"",
 		defaultMaxBodyLineLength,
 		defaultMaxSubjectLineLength)
 }
@@ -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)
 	}
@@ -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 {
@@ -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")