Skip to content

Commit 4d538b2

Browse files
thepuddsgopherbot
authored andcommitted
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 <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Carlos Amedee <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
1 parent 5ba3d51 commit 4d538b2

File tree

3 files changed

+25
-2
lines changed

3 files changed

+25
-2
lines changed

cmd/gerritbot/internal/rules/rules.go

+16-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,15 @@
66
// for certain common mistakes, like no package in the first line of the commit message
77
// or having long lines in the commit message body.
88
//
9+
// The rules attempt to be graceful when encountering a new or unknown repo.
10+
// A new repo usually does not require updating anything here, especially
11+
// if the repo primarily contains Go code and follows typical patterns like
12+
// using the main Go issue tracker. If a new repo is unusual in some way that
13+
// causes a notable problem, some simple options include:
14+
// - adding the repo to skipAll to ignore the repo entirely
15+
// - adding the repo to the skip field on individiual rules
16+
// - updating the usesTracker and packageExample functions if needed
17+
//
918
// A rule is primarily defined via a function that takes a Change (CL or PR) as input
1019
// and reports zero or 1 findings, which is just a string (usually 1-2 short sentences).
1120
// A rule can also optionally return a note, which might be auxiliary advice such as
@@ -72,6 +81,9 @@ type rule struct {
7281
only []string
7382
}
7483

84+
// skipAll lists repos that we ignore entirely by skipping all rules.
85+
var skipAll = []string{"wiki"}
86+
7587
// ruleGroups defines our live set of rules. It is a [][]rule
7688
// because the individual rules are arranged into groups of rules
7789
// that are mutually exclusive, where the first triggering rule wins
@@ -404,7 +416,7 @@ func bugExamples(repo string) string {
404416

405417
// packageExample returns an example usage of a package/component in a commit title
406418
// for a given repo, along with what to call the leading words before the first
407-
// colon ("package" vs. "component").
419+
// colon (e.g., "package" vs. "component").
408420
func packageExample(repo string) (component string, example string) {
409421
switch repo {
410422
default:
@@ -413,6 +425,8 @@ func packageExample(repo string) (component string, example string) {
413425
return "component", "_content/doc/go1.21: fix [...]"
414426
case "vscode-go":
415427
return "component", "src/goInstallTools: improve [...]"
428+
case "wiki":
429+
return "page name or component", "MinimumRequirements: update [...]"
416430
}
417431
}
418432

@@ -439,7 +453,7 @@ func usesTracker(repo string) tracker {
439453
// we also leave unspecified for now.
440454
switch repo {
441455
case "go", "arch", "build", "crypto", "debug", "exp", "image", "mobile", "mod", "net", "perf", "pkgsite", "playground",
442-
"proposal", "review", "sync", "sys", "telemetry", "term", "text", "time", "tools", "tour", "vuln", "website", "xerrors":
456+
"proposal", "review", "sync", "sys", "telemetry", "term", "text", "time", "tools", "tour", "vuln", "website", "wiki", "xerrors":
443457
return mainTracker
444458
case "vscode-go", "oscar":
445459
return ownTracker

cmd/gerritbot/internal/rules/rules_test.go

+6
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ The commit title and commit message body come from the GitHub PR title and descr
4646
The commit title and commit message body come from the GitHub PR title and description, and must be edited in the GitHub web interface (not via git). For instructions, see [here](https://go.dev/wiki/GerritBot/#how-does-gerritbot-determine-the-final-commit-message). For guidelines on commit messages for the Go project, see [here](https://go.dev/doc/contribute#commit_messages).
4747
`,
4848
},
49+
{
50+
title: `A bad wiki commit title we allow`, // We ignore the wiki repo.
51+
repo: "wiki",
52+
body: "A bad body we allow",
53+
want: "",
54+
},
4955
{
5056
title: goodCommitTitle,
5157
body: "This commit body is missing a bug reference.",

cmd/gerritbot/internal/rules/run.go

+3
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ type Result struct {
8181

8282
// Check runs the defined rules against one Change.
8383
func Check(change Change) (results []Result) {
84+
if slices.Contains(skipAll, change.Repo) {
85+
return nil
86+
}
8487
for _, group := range ruleGroups {
8588
for _, rule := range group {
8689
if slices.Contains(rule.skip, change.Repo) || len(rule.only) > 0 && !slices.Contains(rule.only, change.Repo) {

0 commit comments

Comments
 (0)