Repository: jenkins-x/lighthouse
Pull request #1038 (2020-09-10 20:18:09)

refactor: milestone plugin
This PR refactors the milestone plugin.

The `Milestone` service doesn't exist in `go-scm` though, not sure if this plugin works ?

jenkins-x-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **eddycharly**
You can assign the PR to them by writing `/assign @eddycharly` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=jenkins-x%2Flighthouse).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/jenkins-x/lighthouse/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["eddycharly"]} -->
hferentschik: > The Milestone service doesn't exist in go-scm though, not sure if this plugin works ?

If it does not work, I'd rather remove this altogether? @abayer wdyt?
abayer: I'd lean towards actually implementing milestones (in GitHub and Gitea, epics in GitLab) in go-scm. I might do that today.
jenkins-x-bot: @eddycharly: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
github | caeac58a5825a318e92e5989a7dce22e28ca51a1 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/15) | `/test github`

[View all Builds for this Pull Request](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038) 

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
jenkins-x-bot: @eddycharly: The following tests **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
github | caeac58a5825a318e92e5989a7dce22e28ca51a1 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/15) | `/test github`
pr-build | caeac58a5825a318e92e5989a7dce22e28ca51a1 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/13) | `/test pr-build`

[View all Builds for this Pull Request](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038) 

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
abayer: /retest
jenkins-x-bot: @eddycharly: The following tests **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
github | caeac58a5825a318e92e5989a7dce22e28ca51a1 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/15) | `/test github`
pr-build | caeac58a5825a318e92e5989a7dce22e28ca51a1 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/13) | `/test pr-build`
bbs | 146ea0a40076b9dab4ece18ea7752df7f4406214 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/10) | `/test bbs`

[View all Builds for this Pull Request](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038) 

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
jenkins-x-bot: @eddycharly: The following tests **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pr-build | caeac58a5825a318e92e5989a7dce22e28ca51a1 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/13) | `/test pr-build`
bbs | 146ea0a40076b9dab4ece18ea7752df7f4406214 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/10) | `/test bbs`
gitlab | 146ea0a40076b9dab4ece18ea7752df7f4406214 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038/11) | `/test gitlab`

[View all Builds for this Pull Request](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1038) 

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
hferentschik: > I'd lean towards actually implementing milestones in go-scm. I might do that today.

so it is a concept available across scm providers? Fair enough.

Patch:
diff --git a/pkg/plugins/assign/assign.go b/pkg/plugins/assign/assign.go
index 56d6ed94..f59e6e63 100644
--- a/pkg/plugins/assign/assign.go
+++ b/pkg/plugins/assign/assign.go
@@ -32,17 +32,18 @@ import (
 const pluginName = "assign"
 
 var (
-	assignRe = regexp.MustCompile(`(?mi)^/(?:lh-)?(un)?assign(( @?(?:")?[-\w]+?)*(?:")?)\s*$`)
-	// CCRegexp parses and validates /cc commands, also used by blunderbuss
-	CCRegexp = regexp.MustCompile(`(?mi)^/(?:lh-)?(un)?cc(( +@?(?:")?[-/\w]+?)*(?:")?)\s*$`)
+	match = regexp.MustCompile(`(?mi)^/(?:lh-)?(un)?(cc|assign)(( @?(?:")?[-/\w]+?)*(?:")?)\s*$`)
 )
 
 var (
 	plugin = plugins.Plugin{
 		Description: "The assign plugin assigns or requests reviews from users. Specific users can be assigned with the command '/assign @user1' or have reviews requested of them with the command '/cc @user1'. If no user is specified the commands default to targeting the user who created the command. Assignments and requested reviews can be removed in the same way that they are added by prefixing the commands with 'un'.",
 		Commands: []plugins.Command{{
-			GenericCommentHandler: handleGenericComment,
-			Filter:                func(e scmprovider.GenericCommentEvent) bool { return e.Action == scm.ActionCreate },
+			Filter: func(e scmprovider.GenericCommentEvent) bool { return e.Action == scm.ActionCreate },
+			Regex:  match,
+			GenericCommentHandler: func(match []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+				return handleGenericComment(match[1] != "un", match[2], match[3], pc, e)
+			},
 			Help: []pluginhelp.Command{{
 				Usage:       "/[un]assign [[@]<username>...]",
 				Description: "Assigns an assignee to the PR",
@@ -75,10 +76,10 @@ type scmProviderClient interface {
 	QuoteAuthorForComment(string) string
 }
 
-func handleGenericComment(_ []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
-	err := handle(newAssignHandler(e, pc.SCMProviderClient, pc.Logger))
+func handleGenericComment(add bool, command string, arg string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+	err := handle(add, command, arg, newAssignHandler(e, pc.SCMProviderClient, pc.Logger))
 	if e.IsPR {
-		err = combineErrors(err, handle(newReviewHandler(e, pc.SCMProviderClient, pc.Logger)))
+		err = combineErrors(err, handle(add, command, arg, newReviewHandler(e, pc.SCMProviderClient, pc.Logger)))
 	}
 	return err
 }
@@ -109,25 +110,22 @@ func combineErrors(err1, err2 error) error {
 // functions to identify the users to add and/or remove and then passes the appropriate users to the
 // handler's add and remove functions. If add fails to add some of the users, a response comment is
 // created where the body of the response is generated by the handler's addFailureResponse function.
-func handle(h *handler) error {
+func handle(add bool, command string, arg string, h *handler) error {
 	e := h.event
 	org := e.Repo.Namespace
 	repo := e.Repo.Name
-	matches := h.regexp.FindAllStringSubmatch(e.Body, -1)
-	if matches == nil {
+	if command != h.command {
 		return nil
 	}
 	users := make(map[string]bool)
-	for _, re := range matches {
-		add := re[1] != "un" // un<cmd> == !add
-		if re[2] == "" {
-			users[e.Author.Login] = add
-		} else {
-			for _, login := range parseLogins(re[2]) {
-				users[login] = add
-			}
+	if arg == "" {
+		users[e.Author.Login] = add
+	} else {
+		for _, login := range parseLogins(arg) {
+			users[login] = add
 		}
 	}
+
 	var toAdd, toRemove []string
 	for login, add := range users {
 		if add {
@@ -174,9 +172,8 @@ type handler struct {
 
 	// event is a pointer to the gitprovider.GenericCommentEvent struct that triggered the handler.
 	event *scmprovider.GenericCommentEvent
-	// regexp is the regular expression describing the command. It must have an optional 'un' prefix
-	// as the first subgroup and the arguments to the command as the second subgroup.
-	regexp *regexp.Regexp
+	// command is the name of the command to be executed (assign or cc).
+	command string
 	// spc is the scmProviderClient to use for creating response comments in the event of a failure.
 	spc scmProviderClient
 
@@ -197,7 +194,7 @@ func newAssignHandler(e scmprovider.GenericCommentEvent, spc scmProviderClient,
 		remove:             spc.UnassignIssue,
 		add:                spc.AssignIssue,
 		event:              &e,
-		regexp:             assignRe,
+		command:            "assign",
 		spc:                spc,
 		log:                log,
 		userType:           "assignee(s)",
@@ -215,7 +212,7 @@ func newReviewHandler(e scmprovider.GenericCommentEvent, spc scmProviderClient,
 		remove:             spc.UnrequestReview,
 		add:                spc.RequestReview,
 		event:              &e,
-		regexp:             CCRegexp,
+		command:            "cc",
 		spc:                spc,
 		log:                log,
 		userType:           "reviewer(s)",
diff --git a/pkg/plugins/lgtm/lgtm.go b/pkg/plugins/lgtm/lgtm.go
index 3fe7055c..2fd460b3 100644
--- a/pkg/plugins/lgtm/lgtm.go
+++ b/pkg/plugins/lgtm/lgtm.go
@@ -44,8 +44,7 @@ var (
 	configInfoStoreTreeHash    = `Squashing commits does not remove LGTM.`
 	// LGTMLabel is the name of the lgtm label applied by the lgtm plugin
 	LGTMLabel           = labels.LGTM
-	lgtmRe              = regexp.MustCompile(`(?mi)^/(?:lh-)?lgtm(?: no-issue)?\s*$`)
-	lgtmCancelRe        = regexp.MustCompile(`(?mi)^/(?:lh-)?lgtm cancel\s*$`)
+	match               = regexp.MustCompile(`(?mi)^/(?:lh-)?lgtm(?: no-issue)?(?:\s+(cancel))?\s*$`)
 	removeLGTMLabelNoti = "New changes are detected. LGTM label has been removed."
 )
 
@@ -69,8 +68,14 @@ var (
 			Filter: func(e scmprovider.GenericCommentEvent) bool {
 				return !(!e.IsPR || e.IssueState != "open" || e.Action != scm.ActionCreate)
 			},
-			// No regex, the handler will take care of it
-			GenericCommentHandler: handleGenericCommentEvent,
+			Regex: match,
+			GenericCommentHandler: func(m []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+				cp, err := pc.CommentPruner()
+				if err != nil {
+					return err
+				}
+				return handleGenericComment(m[1] == "cancel", pc.SCMProviderClient, pc.PluginConfig, pc.OwnersClient, pc.Logger, cp, e)
+			},
 			Help: []pluginhelp.Command{{
 				Usage:       "/lgtm [cancel] or GitHub Review action",
 				Description: "Adds or removes the 'lgtm' label which is typically used to gate merging.",
@@ -168,14 +173,6 @@ type reviewCtx struct {
 	number                             int
 }
 
-func handleGenericCommentEvent(_ []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
-	cp, err := pc.CommentPruner()
-	if err != nil {
-		return err
-	}
-	return handleGenericComment(pc.SCMProviderClient, pc.PluginConfig, pc.OwnersClient, pc.Logger, cp, e)
-}
-
 func handlePullRequestEvent(pc plugins.Agent, pre scm.PullRequestHook) error {
 	return handlePullRequest(
 		pc.Logger,
@@ -198,7 +195,7 @@ func handlePullRequestReviewEvent(pc plugins.Agent, e scm.ReviewHook) error {
 	return handlePullRequestReview(pc.SCMProviderClient, pc.PluginConfig, pc.OwnersClient, pc.Logger, cp, e)
 }
 
-func handleGenericComment(spc scmProviderClient, config *plugins.Configuration, ownersClient repoowners.Interface, log *logrus.Entry, cp commentPruner, e scmprovider.GenericCommentEvent) error {
+func handleGenericComment(cancel bool, spc scmProviderClient, config *plugins.Configuration, ownersClient repoowners.Interface, log *logrus.Entry, cp commentPruner, e scmprovider.GenericCommentEvent) error {
 	rc := reviewCtx{
 		author:      e.Author.Login,
 		issueAuthor: e.IssueAuthor.Login,
@@ -211,14 +208,7 @@ func handleGenericComment(spc scmProviderClient, config *plugins.Configuration,
 
 	// If we create an "/lgtm" comment, add lgtm if necessary.
 	// If we create a "/lgtm cancel" comment, remove lgtm if necessary.
-	wantLGTM := false
-	if lgtmRe.MatchString(rc.body) {
-		wantLGTM = true
-	} else if lgtmCancelRe.MatchString(rc.body) {
-		wantLGTM = false
-	} else {
-		return nil
-	}
+	wantLGTM := !cancel
 
 	// use common handler to do the rest
 	return handle(wantLGTM, config, ownersClient, rc, spc, log, cp)
@@ -242,7 +232,7 @@ func handlePullRequestReview(spc scmProviderClient, config *plugins.Configuratio
 
 	// If the review event body contains an '/lgtm' or '/lgtm cancel' comment,
 	// skip handling the review event
-	if lgtmRe.MatchString(rc.body) || lgtmCancelRe.MatchString(rc.body) {
+	if match.MatchString(rc.body) {
 		return nil
 	}
 
diff --git a/pkg/plugins/lifecycle/close.go b/pkg/plugins/lifecycle/close.go
index 7eaa5e3f..f4e0edca 100644
--- a/pkg/plugins/lifecycle/close.go
+++ b/pkg/plugins/lifecycle/close.go
@@ -18,7 +18,6 @@ package lifecycle
 
 import (
 	"fmt"
-	"regexp"
 
 	"github.com/jenkins-x/go-scm/scm"
 	"github.com/jenkins-x/lighthouse/pkg/scmprovider"
@@ -27,8 +26,6 @@ import (
 	"github.com/jenkins-x/lighthouse/pkg/plugins"
 )
 
-var closeRe = regexp.MustCompile(`(?mi)^/(?:lh-)?close\s*$`)
-
 type closeClient interface {
 	IsCollaborator(owner, repo, login string) (bool, error)
 	CreateComment(owner, repo string, number int, pr bool, comment string) error
@@ -52,15 +49,6 @@ func isActive(gc closeClient, org, repo string, number int, pr bool) (bool, erro
 }
 
 func handleClose(gc closeClient, log *logrus.Entry, e *scmprovider.GenericCommentEvent) error {
-	// Only consider open issues and new comments.
-	if e.IssueState != "open" || e.Action != scm.ActionCreate {
-		return nil
-	}
-
-	if !closeRe.MatchString(e.Body) {
-		return nil
-	}
-
 	org := e.Repo.Namespace
 	repo := e.Repo.Name
 	number := e.Number
diff --git a/pkg/plugins/lifecycle/lifecycle.go b/pkg/plugins/lifecycle/lifecycle.go
index 93cec59b..91937561 100644
--- a/pkg/plugins/lifecycle/lifecycle.go
+++ b/pkg/plugins/lifecycle/lifecycle.go
@@ -31,6 +31,8 @@ import (
 var (
 	lifecycleLabels = []string{labels.LifecycleActive, labels.LifecycleFrozen, labels.LifecycleStale, labels.LifecycleRotten}
 	lifecycleRe     = regexp.MustCompile(`(?mi)^/(?:lh-)?(remove-)?lifecycle (active|frozen|stale|rotten)\s*$`)
+	closeRe         = regexp.MustCompile(`(?mi)^/(?:lh-)?close\s*$`)
+	reopenRe        = regexp.MustCompile(`(?mi)^/(?:lh-)?reopen\s*$`)
 )
 
 const pluginName = "lifecycle"
@@ -39,26 +41,47 @@ var (
 	plugin = plugins.Plugin{
 		Description: "Close, reopen, flag and/or unflag an issue or PR as frozen/stale/rotten",
 		Commands: []plugins.Command{{
-			Filter:                func(e scmprovider.GenericCommentEvent) bool { return e.Action == scm.ActionCreate },
-			GenericCommentHandler: lifecycleHandleGenericComment,
+			Filter: func(e scmprovider.GenericCommentEvent) bool { return e.Action == scm.ActionCreate },
+			Regex:  lifecycleRe,
+			GenericCommentHandler: func(match []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+				return handleOne(match[1] != "", "lifecycle/"+match[2], pc.SCMProviderClient, pc.Logger, &e)
+			},
+			Help: []pluginhelp.Command{{
+				Usage:       "/[remove-]lifecycle <frozen|stale|rotten>",
+				Description: "Flags an issue or PR as frozen/stale/rotten",
+				Featured:    false,
+				WhoCanUse:   "Anyone can trigger this command.",
+				Examples:    []string{"/lifecycle frozen", "/remove-lifecycle stale", "/lh-lifecyle rotten"},
+			}},
+		}, {
+			Filter: func(e scmprovider.GenericCommentEvent) bool {
+				return e.Action == scm.ActionCreate && e.IssueState == "open"
+			},
+			Regex: closeRe,
+			GenericCommentHandler: func(_ []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+				return handleClose(pc.SCMProviderClient, pc.Logger, &e)
+			},
 			Help: []pluginhelp.Command{{
 				Usage:       "/close",
 				Description: "Closes an issue or PR.",
 				Featured:    false,
 				WhoCanUse:   "Authors and collaborators on the repository can trigger this command.",
 				Examples:    []string{"/close", "/lh-close"},
-			}, {
+			}},
+		}, {
+			Filter: func(e scmprovider.GenericCommentEvent) bool {
+				return e.Action == scm.ActionCreate && e.IssueState == "closed"
+			},
+			Regex: reopenRe,
+			GenericCommentHandler: func(_ []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+				return handleReopen(pc.SCMProviderClient, pc.Logger, &e)
+			},
+			Help: []pluginhelp.Command{{
 				Usage:       "/reopen",
 				Description: "Reopens an issue or PR",
 				Featured:    false,
 				WhoCanUse:   "Authors and collaborators on the repository can trigger this command.",
 				Examples:    []string{"/reopen", "/lh-reopen"},
-			}, {
-				Usage:       "/[remove-]lifecycle <frozen|stale|rotten>",
-				Description: "Flags an issue or PR as frozen/stale/rotten",
-				Featured:    false,
-				WhoCanUse:   "Anyone can trigger this command.",
-				Examples:    []string{"/lifecycle frozen", "/remove-lifecycle stale", "/lh-lifecyle rotten"},
 			}},
 		}},
 	}
@@ -74,36 +97,11 @@ type lifecycleClient interface {
 	GetIssueLabels(org, repo string, number int, pr bool) ([]*scm.Label, error)
 }
 
-func lifecycleHandleGenericComment(_ []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
-	gc := pc.SCMProviderClient
-	log := pc.Logger
-	if err := handleReopen(gc, log, &e); err != nil {
-		return err
-	}
-	if err := handleClose(gc, log, &e); err != nil {
-		return err
-	}
-	return handle(gc, log, &e)
-}
-
-func handle(gc lifecycleClient, log *logrus.Entry, e *scmprovider.GenericCommentEvent) error {
-	for _, mat := range lifecycleRe.FindAllStringSubmatch(e.Body, -1) {
-		if err := handleOne(gc, log, e, mat); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-func handleOne(gc lifecycleClient, log *logrus.Entry, e *scmprovider.GenericCommentEvent, mat []string) error {
+func handleOne(remove bool, lbl string, gc lifecycleClient, log *logrus.Entry, e *scmprovider.GenericCommentEvent) error {
 	org := e.Repo.Namespace
 	repo := e.Repo.Name
 	number := e.Number
 
-	remove := mat[1] != ""
-	cmd := mat[2]
-	lbl := "lifecycle/" + cmd
-
 	// Let's start simple and allow anyone to add/remove frozen, stale, rotten labels.
 	// Adjust if we find evidence of the community abusing these labels.
 	labels, err := gc.GetIssueLabels(org, repo, number, e.IsPR)
diff --git a/pkg/plugins/lifecycle/reopen.go b/pkg/plugins/lifecycle/reopen.go
index 4ec71389..8529e159 100644
--- a/pkg/plugins/lifecycle/reopen.go
+++ b/pkg/plugins/lifecycle/reopen.go
@@ -18,7 +18,6 @@ package lifecycle
 
 import (
 	"fmt"
-	"regexp"
 
 	"github.com/jenkins-x/go-scm/scm"
 	"github.com/jenkins-x/lighthouse/pkg/scmprovider"
@@ -27,8 +26,6 @@ import (
 	"github.com/jenkins-x/lighthouse/pkg/plugins"
 )
 
-var reopenRe = regexp.MustCompile(`(?mi)^/(?:lh-)?reopen\s*$`)
-
 type scmProviderClient interface {
 	IsCollaborator(owner, repo, login string) (bool, error)
 	CreateComment(owner, repo string, number int, pr bool, comment string) error
@@ -38,15 +35,6 @@ type scmProviderClient interface {
 }
 
 func handleReopen(spc scmProviderClient, log *logrus.Entry, e *scmprovider.GenericCommentEvent) error {
-	// Only consider closed issues and new comments.
-	if e.IssueState != "closed" || e.Action != scm.ActionCreate {
-		return nil
-	}
-
-	if !reopenRe.MatchString(e.Body) {
-		return nil
-	}
-
 	org := e.Repo.Namespace
 	repo := e.Repo.Name
 	number := e.Number
diff --git a/pkg/plugins/milestone/milestone.go b/pkg/plugins/milestone/milestone.go
index 61643c32..836757a3 100644
--- a/pkg/plugins/milestone/milestone.go
+++ b/pkg/plugins/milestone/milestone.go
@@ -56,9 +56,11 @@ var (
 		Description:        "The milestone plugin allows members of a configurable GitHub team to set the milestone on an issue or pull request.",
 		ConfigHelpProvider: configHelp,
 		Commands: []plugins.Command{{
-			GenericCommentHandler: handleGenericComment,
-			Filter:                func(e scmprovider.GenericCommentEvent) bool { return e.Action == scm.ActionCreate },
-			Regex:                 milestoneRegex,
+			GenericCommentHandler: func(match []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+				return handle(match[1], pc.SCMProviderClient, pc.Logger, &e, pc.PluginConfig.RepoMilestone)
+			},
+			Filter: func(e scmprovider.GenericCommentEvent) bool { return e.Action == scm.ActionCreate },
+			Regex:  milestoneRegex,
 			Help: []pluginhelp.Command{{
 				Usage:       "/milestone <version> or /milestone clear",
 				Description: "Updates the milestone for an issue or PR",
@@ -89,10 +91,6 @@ func configHelp(config *plugins.Configuration, enabledRepos []string) (map[strin
 	return configMap, nil
 }
 
-func handleGenericComment(match []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
-	return handle(match, pc.SCMProviderClient, pc.Logger, &e, pc.PluginConfig.RepoMilestone)
-}
-
 func buildMilestoneMap(milestones []scmprovider.Milestone) map[string]int {
 	m := make(map[string]int)
 	for _, ms := range milestones {
@@ -101,7 +99,7 @@ func buildMilestoneMap(milestones []scmprovider.Milestone) map[string]int {
 	return m
 }
 
-func handle(match []string, spc scmProviderClient, log *logrus.Entry, e *scmprovider.GenericCommentEvent, repoMilestone map[string]plugins.Milestone) error {
+func handle(mileStone string, spc scmProviderClient, log *logrus.Entry, e *scmprovider.GenericCommentEvent, repoMilestone map[string]plugins.Milestone) error {
 	org := e.Repo.Namespace
 	repo := e.Repo.Name
 
@@ -134,10 +132,9 @@ func handle(match []string, spc scmProviderClient, log *logrus.Entry, e *scmprov
 		log.WithError(err).Errorf("Error listing the milestones in the %s/%s repo", org, repo)
 		return err
 	}
-	proposedMilestone := match[1]
 
 	// special case, if the clear keyword is used
-	if proposedMilestone == clearKeyword {
+	if mileStone == clearKeyword {
 		if err := spc.ClearMilestone(org, repo, e.Number); err != nil {
 			log.WithError(err).Errorf("Error clearing the milestone for %s/%s#%d.", org, repo, e.Number)
 		}
@@ -145,7 +142,7 @@ func handle(match []string, spc scmProviderClient, log *logrus.Entry, e *scmprov
 	}
 
 	milestoneMap := buildMilestoneMap(milestones)
-	milestoneNumber, ok := milestoneMap[proposedMilestone]
+	milestoneNumber, ok := milestoneMap[mileStone]
 	if !ok {
 		slice := make([]string, 0, len(milestoneMap))
 		for k := range milestoneMap {
@@ -158,7 +155,7 @@ func handle(match []string, spc scmProviderClient, log *logrus.Entry, e *scmprov
 	}
 
 	if err := spc.SetMilestone(org, repo, e.Number, milestoneNumber); err != nil {
-		log.WithError(err).Errorf("Error adding the milestone %s to %s/%s#%d.", proposedMilestone, org, repo, e.Number)
+		log.WithError(err).Errorf("Error adding the milestone %s to %s/%s#%d.", mileStone, org, repo, e.Number)
 	}
 
 	return nil


Test patch:
diff --git a/pkg/plugins/assign/assign_test.go b/pkg/plugins/assign/assign_test.go
index ff6ef178..0acd6a82 100644
--- a/pkg/plugins/assign/assign_test.go
+++ b/pkg/plugins/assign/assign_test.go
@@ -413,16 +413,24 @@ func TestAssignAndReview(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			fc := newFakeClient([]string{"hello-world", "allow_underscore", "cjwagner", "merlin", "kubernetes/sig-testing-misc"})
 			e := scmprovider.GenericCommentEvent{
+				Action: scm.ActionCreate,
 				Body:   tc.body,
 				Author: scm.User{Login: tc.commenter},
 				Repo:   scm.Repository{Name: "repo", Namespace: "org"},
 				Number: 5,
 			}
-			if err := handle(newAssignHandler(e, fc, logrus.WithField("plugin", pluginName))); err != nil {
-				t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+			cmd := plugin.Commands[0]
+			matches, err := cmd.GetMatches(&e)
+			if err != nil {
+				t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
 			}
-			if err := handle(newReviewHandler(e, fc, logrus.WithField("plugin", pluginName))); err != nil {
-				t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+			for _, m := range matches {
+				if err := handle(m[1] != "un", m[2], m[3], newAssignHandler(e, fc, logrus.WithField("plugin", pluginName))); err != nil {
+					t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+				}
+				if err := handle(m[1] != "un", m[2], m[3], newReviewHandler(e, fc, logrus.WithField("plugin", pluginName))); err != nil {
+					t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+				}
 			}
 
 			if tc.commented != fc.commented {
diff --git a/pkg/plugins/lgtm/lgtm_test.go b/pkg/plugins/lgtm/lgtm_test.go
index eebb601b..296ef8ce 100644
--- a/pkg/plugins/lgtm/lgtm_test.go
+++ b/pkg/plugins/lgtm/lgtm_test.go
@@ -348,8 +348,15 @@ func TestLGTMComment(t *testing.T) {
 				SCMProviderClient:   fc,
 				PullRequestComments: fc.PullRequestComments[5],
 			}
-			if err := handleGenericComment(fakeClient, pc, oc, logrus.WithField("plugin", pluginName), fp, *e); err != nil {
-				t.Fatalf("didn't expect error from lgtmComment: %v", err)
+			cmd := plugin.Commands[0]
+			matches, err := cmd.GetMatches(e)
+			if err != nil {
+				t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
+			}
+			for _, m := range matches {
+				if err := handleGenericComment(m[1] == "cancel", fakeClient, pc, oc, logrus.WithField("plugin", pluginName), fp, *e); err != nil {
+					t.Fatalf("For case %s, didn't expect error from label test: %v", tc.name, err)
+				}
 			}
 			if err := fakeClient.PopulateFakeLabelsFromComments("org", "repo", 5, fakeLabel, tc.hasLGTM && tc.shouldToggle); err != nil {
 				t.Fatalf("Failure populating labels from comments: %v", err)
@@ -509,9 +516,16 @@ func TestLGTMCommentWithLGTMNoti(t *testing.T) {
 			SCMProviderClient:   fc,
 			PullRequestComments: fc.PullRequestComments[5],
 		}
-		if err := handleGenericComment(fakeClient, pc, oc, logrus.WithField("plugin", pluginName), fp, *e); err != nil {
-			t.Errorf("For case %s, didn't expect error from lgtmComment: %v", tc.name, err)
-			continue
+		cmd := plugin.Commands[0]
+		matches, err := cmd.GetMatches(e)
+		if err != nil {
+			t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
+		}
+		for _, m := range matches {
+			if err := handleGenericComment(m[1] == "cancel", fakeClient, pc, oc, logrus.WithField("plugin", pluginName), fp, *e); err != nil {
+				t.Errorf("For case %s, didn't expect error from lgtmComment: %v", tc.name, err)
+				continue
+			}
 		}
 		deleted := false
 		for _, body := range fc.PullRequestCommentsDeleted {
diff --git a/pkg/plugins/lifecycle/close_test.go b/pkg/plugins/lifecycle/close_test.go
index 57c42865..55ed2dd0 100644
--- a/pkg/plugins/lifecycle/close_test.go
+++ b/pkg/plugins/lifecycle/close_test.go
@@ -194,8 +194,15 @@ func TestCloseComment(t *testing.T) {
 				Number:      5,
 				IssueAuthor: scm.User{Login: "author"},
 			}
-			if err := handleClose(fc, logrus.WithField("plugin", "fake-close"), e); err != nil {
-				t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+			cmd := plugin.Commands[1]
+			matches, err := cmd.GetMatches(e)
+			if err != nil {
+				t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
+			}
+			for range matches {
+				if err := handleClose(fc, logrus.WithField("plugin", pluginName), e); err != nil {
+					t.Fatalf("For case %s, didn't expect error from label test: %v", tc.name, err)
+				}
 			}
 			if tc.shouldClose && !fc.closed {
 				t.Errorf("For case %s, should have closed but didn't.", tc.name)
diff --git a/pkg/plugins/lifecycle/lifecycle_test.go b/pkg/plugins/lifecycle/lifecycle_test.go
index 4a6517d9..2426f01e 100644
--- a/pkg/plugins/lifecycle/lifecycle_test.go
+++ b/pkg/plugins/lifecycle/lifecycle_test.go
@@ -231,7 +231,16 @@ func TestAddLifecycleLabels(t *testing.T) {
 				Body:   tc.body,
 				Action: scm.ActionCreate,
 			}
-			err := handle(fc, logrus.WithField("plugin", "fake-lifecyle"), e)
+			cmd := plugin.Commands[0]
+			matches, err := cmd.GetMatches(e)
+			if err != nil {
+				t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
+			}
+			for _, m := range matches {
+				if err := handleOne(m[1] != "", "lifecycle/"+m[2], fc, logrus.WithField("plugin", pluginName), e); err != nil {
+					t.Fatalf("For case %s, didn't expect error from label test: %v", tc.name, err)
+				}
+			}
 			switch {
 			case err != nil:
 				t.Errorf("%s: unexpected error: %v", tc.name, err)
diff --git a/pkg/plugins/lifecycle/reopen_test.go b/pkg/plugins/lifecycle/reopen_test.go
index 3b95d1cd..2131d92e 100644
--- a/pkg/plugins/lifecycle/reopen_test.go
+++ b/pkg/plugins/lifecycle/reopen_test.go
@@ -149,8 +149,15 @@ func TestReopenComment(t *testing.T) {
 				Number:      5,
 				IssueAuthor: scm.User{Login: "author"},
 			}
-			if err := handleReopen(fc, logrus.WithField("plugin", "fake-reopen"), e); err != nil {
-				t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+			cmd := plugin.Commands[2]
+			matches, err := cmd.GetMatches(e)
+			if err != nil {
+				t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
+			}
+			for range matches {
+				if err := handleReopen(fc, logrus.WithField("plugin", pluginName), e); err != nil {
+					t.Fatalf("For case %s, didn't expect error from label test: %v", tc.name, err)
+				}
 			}
 			if tc.shouldReopen && !fc.open {
 				t.Errorf("For case %s, should have reopened but didn't.", tc.name)
diff --git a/pkg/plugins/milestone/milestone_test.go b/pkg/plugins/milestone/milestone_test.go
index 8ccb0d07..9b1045af 100644
--- a/pkg/plugins/milestone/milestone_test.go
+++ b/pkg/plugins/milestone/milestone_test.go
@@ -145,11 +145,16 @@ func TestMilestoneStatus(t *testing.T) {
 				repoMilestone["org/repo"] = plugins.Milestone{MaintainersID: maintainersID, MaintainersTeam: maintainersName}
 			}
 
-			if err := plugin.InvokeCommand(e, func(match []string) error {
-				return handle(match, fakeClient, logrus.WithField("plugin", pluginName), e, repoMilestone)
-			}); err != nil {
+			cmd := plugin.Commands[0]
+			matches, err := cmd.GetMatches(e)
+			if err != nil {
 				t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
 			}
+			for _, m := range matches {
+				if err := handle(m[1], fakeClient, logrus.WithField("plugin", pluginName), e, repoMilestone); err != nil {
+					t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
+				}
+			}
 
 			// Check that the milestone was set if it was supposed to be set
 			if fakeClient.Milestone != tc.expectedMilestone {