cmd/gerritbot/internal/rules: skip checking the wiki repo to minimize friction
Core team reviewers usually seem to approve wiki CLs and PRs without
asking for any changes to a commit message even if it does not conform
to general CL guidelines for the main Go repos.
For example, CL 587895 was approved with the commit title of:
add "cue" as a well-known struct tag
(without a leading "wiki:" or leading page name or similar).
This seems fine, including I think the general intent was to keep the
reviews very lightweight when the wiki content was moved
(golang/go#61940).
Currently, GerritBot is too picky with wiki PRs.
This CL therefore updates GerritBot to avoid flagging any issues with
wiki PRs. (An alternative would be to skip just some rules, but it
seems simpler to skip all).
Partly in case there is a later decision to start flagging wiki PRs,
while we are here we update the packageExample and usesTracker functions
to be more specific about the wiki repo (which won't have any effect
right now because the wiki repo is now ignored).
Fixes golang/go#65281
Change-Id: I7f678a9f997f35fa048b78aab1ee4a6d808b4ec6
Reviewed-on: https://go-review.googlesource.com/c/build/+/640135
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
diff --git a/cmd/gerritbot/internal/rules/rules.go b/cmd/gerritbot/internal/rules/rules.go
index e4b084a..72234b9 100644
--- a/cmd/gerritbot/internal/rules/rules.go
+++ b/cmd/gerritbot/internal/rules/rules.go
@@ -6,6 +6,15 @@
// for certain common mistakes, like no package in the first line of the commit message
// or having long lines in the commit message body.
//
+// The rules attempt to be graceful when encountering a new or unknown repo.
+// A new repo usually does not require updating anything here, especially
+// if the repo primarily contains Go code and follows typical patterns like
+// using the main Go issue tracker. If a new repo is unusual in some way that
+// causes a notable problem, some simple options include:
+// - adding the repo to skipAll to ignore the repo entirely
+// - adding the repo to the skip field on individiual rules
+// - updating the usesTracker and packageExample functions if needed
+//
// A rule is primarily defined via a function that takes a Change (CL or PR) as input
// and reports zero or 1 findings, which is just a string (usually 1-2 short sentences).
// A rule can also optionally return a note, which might be auxiliary advice such as
@@ -72,6 +81,9 @@
only []string
}
+// skipAll lists repos that we ignore entirely by skipping all rules.
+var skipAll = []string{"wiki"}
+
// ruleGroups defines our live set of rules. It is a [][]rule
// because the individual rules are arranged into groups of rules
// that are mutually exclusive, where the first triggering rule wins
@@ -404,7 +416,7 @@
// packageExample returns an example usage of a package/component in a commit title
// for a given repo, along with what to call the leading words before the first
-// colon ("package" vs. "component").
+// colon (e.g., "package" vs. "component").
func packageExample(repo string) (component string, example string) {
switch repo {
default:
@@ -413,6 +425,8 @@
return "component", "_content/doc/go1.21: fix [...]"
case "vscode-go":
return "component", "src/goInstallTools: improve [...]"
+ case "wiki":
+ return "page name or component", "MinimumRequirements: update [...]"
}
}
@@ -439,7 +453,7 @@
// we also leave unspecified for now.
switch repo {
case "go", "arch", "build", "crypto", "debug", "exp", "image", "mobile", "mod", "net", "perf", "pkgsite", "playground",
- "proposal", "review", "sync", "sys", "telemetry", "term", "text", "time", "tools", "tour", "vuln", "website", "xerrors":
+ "proposal", "review", "sync", "sys", "telemetry", "term", "text", "time", "tools", "tour", "vuln", "website", "wiki", "xerrors":
return mainTracker
case "vscode-go", "oscar":
return ownTracker
diff --git a/cmd/gerritbot/internal/rules/rules_test.go b/cmd/gerritbot/internal/rules/rules_test.go
index f95621c..25af204 100644
--- a/cmd/gerritbot/internal/rules/rules_test.go
+++ b/cmd/gerritbot/internal/rules/rules_test.go
@@ -47,6 +47,12 @@
`,
},
{
+ title: `A bad wiki commit title we allow`, // We ignore the wiki repo.
+ repo: "wiki",
+ body: "A bad body we allow",
+ want: "",
+ },
+ {
title: goodCommitTitle,
body: "This commit body is missing a bug reference.",
want: `Possible problems detected:
diff --git a/cmd/gerritbot/internal/rules/run.go b/cmd/gerritbot/internal/rules/run.go
index 4974516..a5e4603 100644
--- a/cmd/gerritbot/internal/rules/run.go
+++ b/cmd/gerritbot/internal/rules/run.go
@@ -81,6 +81,9 @@
// Check runs the defined rules against one Change.
func Check(change Change) (results []Result) {
+ if slices.Contains(skipAll, change.Repo) {
+ return nil
+ }
for _, group := range ruleGroups {
for _, rule := range group {
if slices.Contains(rule.skip, change.Repo) || len(rule.only) > 0 && !slices.Contains(rule.only, change.Repo) {
