diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d97ecf640a9e..cdf2a08dde8a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -332,7 +332,6 @@ /pkg/cmd/gossipsim/ @cockroachdb/kv-prs /pkg/cmd/import-tools/ @cockroachdb/dev-inf /pkg/cmd/internal/issues/ @cockroachdb/test-eng -/pkg/cmd/lint-epic-issue-refs/ @cockroachdb/dev-inf /pkg/cmd/mirror/ @cockroachdb/dev-inf /pkg/cmd/prereqs/ @cockroachdb/dev-inf /pkg/cmd/protoc-gen-gogoroach/ @cockroachdb/dev-inf diff --git a/.github/workflows/pr-epic-issue-ref-lint.yml b/.github/workflows/pr-epic-issue-ref-lint.yml deleted file mode 100644 index 8e22fe20d124..000000000000 --- a/.github/workflows/pr-epic-issue-ref-lint.yml +++ /dev/null @@ -1,35 +0,0 @@ -name: Epic + Issue Ref Linter for PR Body or Commit Messages - -on: - pull_request: - types: [opened, reopened, synchronize, edited] - -jobs: - linter: - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v3 - - - name: Bazel cache - id: bazel-cache - uses: actions/cache@v3 - with: - path: | - ~/.cache/bazel - key: ${{ runner.os }}-bazel-cache - - - name: Install bazelisk - run: | - curl -fsSL https://github.com/bazelbuild/bazelisk/releases/download/v1.10.1/bazelisk-linux-amd64 > /tmp/bazelisk - sha256sum -c - < 0 { - for _, x := range allMatches { - matches := secondMatch.FindAllString(x, -1) - for _, match := range matches { - ids[match]++ - } - } - } - return ids -} - -func extractFixIssueIDs(message string) map[string]int { - return extractStringsFromMessage(message, fixIssueRefRE, githubJiraIssueRefRE) -} - -func extractInformIssueIDs(message string) map[string]int { - return extractStringsFromMessage(message, informIssueRefRE, githubJiraIssueRefRE) -} - -func extractEpicIDs(message string) map[string]int { - return extractStringsFromMessage(message, epicRefRE, jiraIssueRefRE) -} - -func extractEpicNone(message string) bool { - if allMatches := epicNoneRE.FindAllString(message, -1); len(allMatches) > 0 { - return true - } - return false -} - -func scanCommitsForEpicAndIssueRefs( - ctx context.Context, ghClient *github.Client, params *parameters, -) ([]commitInfo, error) { - commits := []*github.RepositoryCommit{} - for page := 1; page <= 3; page++ { - commitsInQuery, _, err := ghClient.PullRequests.ListCommits( - ctx, params.Org, params.Repo, params.PrNumber, &github.ListOptions{PerPage: 100, Page: page}, - ) - if err != nil { - return nil, err - } - commits = append(commits, commitsInQuery...) - if len(commitsInQuery) < 100 { - break - } - } - - var infos []commitInfo - for _, commit := range commits { - message := commit.GetCommit().GetMessage() - var info = commitInfo{ - epicRefs: extractEpicIDs(message), - epicNone: extractEpicNone(message), - issueCloseRefs: extractFixIssueIDs(message), - issueInformRefs: extractInformIssueIDs(message), - sha: commit.GetSHA(), - } - infos = append(infos, info) - } - - return infos, nil -} - -func commitShaURL(sha string, params *parameters) string { - return fmt.Sprintf("https://github.com/%s/%s/commit/%s", params.Org, params.Repo, sha) -} - -// checkForMissingRefs determines if the PR and its commits has the needed refs. -// When the PR body is missing a ref and one or more commits are missing a ref, tell the -// caller which commits are missing refs. -// -// Returns: -// - True if the check passed. False if it failed. -func checkForMissingRefs(prInfo prBodyInfo, commitInfos []commitInfo, params *parameters) bool { - // When the PR body has a ref, no refs are needed in individual commits - if len(prInfo.epicRefs)+len(prInfo.issueInformRefs)+len(prInfo.issueCloseRefs) > 0 || prInfo.epicNone { - return true - } - - commitsWithoutRefs := []string{} - for _, info := range commitInfos { - if len(info.epicRefs)+len(info.issueInformRefs)+len(info.issueCloseRefs) == 0 && !info.epicNone { - commitsWithoutRefs = append(commitsWithoutRefs, commitShaURL(info.sha, params)) - } - } - if len(commitsWithoutRefs) > 0 { - if len(commitsWithoutRefs) == len(commitInfos) { - fmt.Print("Error: The PR body or each commit in the PR should have an epic or issue ref but they don't\n\n") - } else { - fmt.Printf("Error: These commits should have an epic or issue ref but don't: %v\n\n", commitsWithoutRefs) - } - return false - } - return true -} - -// checkPrEpicsUsedInCommits checks that all epic references in the PR body are used in at least one -// commit message and that the individual commits note the epic with which they are associated. -// -// Returns: -// - True if the check passed. False if it failed. -func checkPrEpicsUsedInCommits( - prInfo prBodyInfo, commitInfos []commitInfo, params *parameters, -) bool { - if len(prInfo.epicRefs) < 2 { - return true - } - - passedCheck := true - printHeader := func() { - if !passedCheck { - return - } - fmt.Print( - "When multiple epics are referenced in the PR body, each commit of the PR should " + - "reference the epic(s) that commit is part of and all epics listed in the commit messages " + - "should be listed in the PR body. These unexpected cases were found:\n", - ) - passedCheck = false - } - - // Check PR body epics are all referenced from commit messages - for _, prEpic := range reflect.ValueOf(prInfo.epicRefs).MapKeys() { - found := false - for _, ci := range commitInfos { - if ci.epicRefs[prEpic.String()] > 0 { - found = true - } - } - if !found { - printHeader() - fmt.Printf( - "- Error: This epic was listed in the PR body but was not referenced in any commits: %s\n", - prEpic.String(), - ) - } - } - - // Check that all commit messages reference one of the PR epics - for _, ci := range commitInfos { - if ci.epicNone { - continue - } - if len(ci.epicRefs) == 0 { - printHeader() - fmt.Printf("- Error: This commit did not reference an epic but should: %#v\n", commitShaURL(ci.sha, params)) - } else { - for _, epicRef := range reflect.ValueOf(ci.epicRefs).MapKeys() { - if _, ok := prInfo.epicRefs[epicRef.String()]; !ok { - printHeader() - fmt.Printf( - "- Error: This commit references an epic that isn't referenced in the PR body. "+ - "epic_ref: %v, commit: %s\n", - epicRef, - commitShaURL(ci.sha, params), - ) - } - } - } - } - - if !passedCheck { - fmt.Println() - } - - return passedCheck -} - -// checkMultipleEpicsInCommits checks for commits that contain multiple epic references and adds a -// warning that it is not a common case and to check it is intended. -// -// Returns: -// - True if the check passed. False if it failed. -func checkMultipleEpicsInCommits(commitInfos []commitInfo, params *parameters) bool { - passedCheck := true - for _, ci := range commitInfos { - if len(ci.epicRefs) > 1 { - fmt.Printf( - "Warning: This commit references multiple epics. Normally a commit only references one epic. "+ - "Noting this to verify this commit relates to multiple epics. "+ - "epic_refs=[%v], commit: %s\n\n", - ci.epicRefs, - commitShaURL(ci.sha, params), - ) - passedCheck = false - } - } - - return passedCheck -} - -func lintEpicAndIssueRefs(ctx context.Context, ghClient *github.Client, params *parameters) error { - commitInfos, err := scanCommitsForEpicAndIssueRefs(ctx, ghClient, params) - if err != nil { - return err - } - - pr, _, err := ghClient.PullRequests.Get( - ctx, params.Org, params.Repo, params.PrNumber, - ) - if err != nil { - // nolint:errwrap - return fmt.Errorf("error getting pull requests from GitHub: %v", err) - } - prBody := pr.GetBody() - - // Skip checking multi-PR backports for now. Verifying them is complex and the source PRs - // have already been checked. - if isMultiPrBackport(prBody) { - // TODO(jamesl): handle backport PRs containing multiple source PRs differently - // How might they be handled? - // - check that all source PR bodies have epic or issue refs? - // - figure out which commits in this PR correspond to the commits in the source PRs and ... - // - check all source PRs have all the expected references a PR should have. Do something - // with that info, like maybe link it back to the commits in this PR and report on it - // somehow??? (This seems really heavy / not useful.) - // - don't check backports with multiple source PRs for the complex cases like - // "checkPrEpicsUsedInCommits"? - // - these checks are really for making it possible for the automated docs issue generation to - // get the right epic(s) applied to them. Maybe make this a super simple check and then add - // info to the generated docs issue stating it needs to be reviewed more. This case shouldn't - // happen too often. The majority of backports are for a single source PR. - fmt.Println("This is a multi-PR backport. Skipping checking it.") - return nil - } - - var prInfo = prBodyInfo{ - epicRefs: extractEpicIDs(prBody), - epicNone: extractEpicNone(prBody), - issueCloseRefs: extractFixIssueIDs(prBody), - issueInformRefs: extractInformIssueIDs(prBody), - prNumber: params.PrNumber, - } - - passedAllTests := checkForMissingRefs(prInfo, commitInfos, params) && - checkPrEpicsUsedInCommits(prInfo, commitInfos, params) && - checkMultipleEpicsInCommits(commitInfos, params) - - if !passedAllTests { - fmt.Println("For more information about issue and epic references, see: https://wiki.crdb.io/wiki/spaces/CRDB/pages/2009039063/") - return errors.New("one or more checks failed; see logs above") - } - - // TODO: - // - add some "try" messages for how they can fix the issue in question. - return nil -} - -func lintPR(params *parameters) error { - ctx := context.Background() - - client := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: params.Token}, - ))) - - return lintEpicAndIssueRefs(ctx, client, params) -} diff --git a/pkg/cmd/lint-epic-issue-refs/lint_epic_issue_refs_test.go b/pkg/cmd/lint-epic-issue-refs/lint_epic_issue_refs_test.go deleted file mode 100644 index 21c81c62c14b..000000000000 --- a/pkg/cmd/lint-epic-issue-refs/lint_epic_issue_refs_test.go +++ /dev/null @@ -1,505 +0,0 @@ -// Copyright 2022 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. -package main - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestExtractFixIssuesIDs(t *testing.T) { - testCases := []struct { - message string - expected map[string]int - }{ - { - message: `workload: Fix folder name generation. - -Fixes #75200 #98922 -close #75201 -closed #592 -RESOLVE #5555 - -Release Notes: None`, - expected: map[string]int{"#75200": 1, "#98922": 1, "#75201": 1, "#592": 1, "#5555": 1}, - }, - { - message: `logictestccl: skip flaky TestCCLLogic/fakedist-metadata/partitioning_enum - -Informs #75227 -Epic CRDB-491 - -Release note (bug fix): Fixin the bug`, - expected: map[string]int{}, - }, - { - message: `lots of variations - -fixes #74932; we were reading from the replicas map without... -Closes #74889. -Resolves #74482, #74784 #65117 #79299. -Fix: #73834 -epic: CRDB-9234. -epic CRDB-235, CRDB-40192; DEVINF-392 -Fixed: #29833 example/repo#941 -see also: #9243 -informs: #912, #4729 #2911 cockroachdb/cockroach#2934 - -Release note (sql change): Import now checks readability...`, - expected: map[string]int{"#74932": 1, "#74889": 1, "#74482": 1, "#74784": 1, "#65117": 1, "#79299": 1, "#73834": 1, "#29833": 1, "example/repo#941": 1}, - }, - { - message: `lots of variations 2 - -Resolved: #4921 -This comes w/ support for Applie Silicon Macs. Closes #72829. -This doesn't completely fix #71901 in that some... - Fixes #491 -Resolves #71614, Resolves #71607 -Thereby, fixes #69765 -Informs #69765 (point 4). -Fixes #65200. The last remaining 21.1 version (V21_1) can be removed as - -Release note (build change): Upgrade to Go 1.17.6 -Release note (ops change): Added a metric -Release notes (enterprise change): Client certificates may...`, - expected: map[string]int{"#4921": 1, "#72829": 1, "#71901": 1, "#491": 1, "#71614": 1, "#71607": 1, "#69765": 1, "#65200": 1}, - }, - { - message: `testing JIRA tickets - -Resolved: doc-4321 -This fixes everything. Closes CC-1234. - Fixes CRDB-12345 -Resolves crdb-23456, Resolves DEVINFHD-12345 -Fixes #12345 -Release notes (sql change): Excellent sql change...`, - expected: map[string]int{"doc-4321": 1, "CC-1234": 1, "CRDB-12345": 1, "crdb-23456": 1, "DEVINFHD-12345": 1, "#12345": 1}, - }, - { - message: `testing URL refs - -Resolves: https://github.com/cockroachlabs/support/issues/1833 -This fixes everything. Closes https://github.com/cockroachdb/cockroach/issues/83912. -Fix: https://cockroachlabs.atlassian.net/browse/DOC-9492 - -Release notes (sql change): Excellent sql change...`, - expected: map[string]int{ - "https://github.com/cockroachlabs/support/issues/1833": 1, - "https://github.com/cockroachdb/cockroach/issues/83912": 1, - "https://cockroachlabs.atlassian.net/browse/DOC-9492": 1, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.message, func(t *testing.T) { - result := extractFixIssueIDs(tc.message) - assert.Equal(t, tc.expected, result) - }) - } -} - -func TestExtractInformIssueIDs(t *testing.T) { - testCases := []struct { - message string - expected map[string]int - }{ - { - message: `logictestccl: skip flaky TestCCLLogic/fakedist-metadata/partitioning_enum - -Informs #75227 -Part of #45791 -Epic CRDB-491 -Fix: #73834 - -Release note (bug fix): Fixin the bug`, - expected: map[string]int{"#75227": 1, "#45791": 1}, - }, - { - message: `lots of variations - -Fixed: #29833 example/repo#941 -see also: #9243 -informs: #912, #4729 #2911 cockroachdb/cockroach#2934 -Informs #69765 (point 4). -This informs #59293 with these additions: - -Release note (sql change): Import now checks readability...`, - expected: map[string]int{"#9243": 1, "#912": 1, "#4729": 1, "#2911": 1, "cockroachdb/cockroach#2934": 1, "#69765": 1, "#59293": 1}, - }, - { - message: `testing JIRA keys with varying cases - -Fixed: CRDB-12345 example/repo#941 -informs: doc-1234, crdb-24680 -Informs DEVINF-123, #69765 and part of DEVINF-3891 - -Release note (sql change): Something something something...`, - expected: map[string]int{"doc-1234": 1, "DEVINF-123": 1, "#69765": 1, "crdb-24680": 1, "DEVINF-3891": 1}, - }, - { - message: `testing URL refs - -Fixed: CRDB-12345 example/repo#941 -informs: https://github.com/cockroachdb/cockroach/issues/8892, https://github.com/cockroachdb/pebble/issues/309 -PART OF https://cockroachlabs.atlassian.net/browse/DOC-3891 - -Release note (sql change): Something something something...`, - expected: map[string]int{ - "https://github.com/cockroachdb/cockroach/issues/8892": 1, - "https://github.com/cockroachdb/pebble/issues/309": 1, - "https://cockroachlabs.atlassian.net/browse/DOC-3891": 1, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.message, func(t *testing.T) { - result := extractInformIssueIDs(tc.message) - assert.Equal(t, tc.expected, result) - }) - } -} - -func TestExtractEpicIDs(t *testing.T) { - testCases := []struct { - message string - expected map[string]int - }{ - { - message: `logictestccl: skip flaky TestCCLLogic/fakedist-metadata/partitioning_enum - -Informs #75227 -Epic CRDB-491 -Fix: #73834 - -Release note (bug fix): Fixin the bug`, - expected: map[string]int{"CRDB-491": 1}, - }, - { - message: `lots of variations - -epic: CRDB-9234. -epic CRDB-235, CRDB-40192; DEVINF-392 https://cockroachlabs.atlassian.net/browse/CRDB-97531 -Epic doc-9238 - -Release note (sql change): Import now checks readability...`, - expected: map[string]int{ - "CRDB-9234": 1, - "CRDB-235": 1, - "CRDB-40192": 1, - "DEVINF-392": 1, - "doc-9238": 1, - "https://cockroachlabs.atlassian.net/browse/CRDB-97531": 1, - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.message, func(t *testing.T) { - result := extractEpicIDs(tc.message) - assert.Equal(t, tc.expected, result) - }) - } -} - -func TestExtractEpicNone(t *testing.T) { - testCases := []struct { - message string - expected bool - }{ - { - message: `logictestccl: skip flaky TestCCLLogic/fakedist-metadata/partitioning_enum - -Epic CRDB-491 -Fix: #73834 - -Release note (bug fix): Fixin the bug`, - expected: false, - }, - { - message: `lots of variations - -epic: None - -Release note (sql change): Import now checks readability...`, - expected: true, - }, - { - message: `another test - -Epic nONE - -Release note (sql change): Import now checks readability...`, - expected: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.message, func(t *testing.T) { - result := extractEpicNone(tc.message) - assert.Equal(t, tc.expected, result) - }) - } -} - -func TestCheckForMissingRefs(t *testing.T) { - testCases := []struct { - message string - prInfo prBodyInfo - commitInfos []commitInfo - expected bool - }{ - { - message: "No refs set anywhere", - prInfo: prBodyInfo{}, - commitInfos: []commitInfo{{sha: "d53a4c0"}}, - expected: false, - }, - { - message: "One commit without refs", - prInfo: prBodyInfo{}, - commitInfos: []commitInfo{{sha: "d53a4c0"}, {sha: "128e2038", epicNone: true}}, - expected: false, - }, - { - message: "Epic ref in PR body", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-4123": 1}}, - commitInfos: []commitInfo{{}}, - expected: true, - }, - { - message: "Epic None set in PR body", - prInfo: prBodyInfo{epicNone: true}, - commitInfos: []commitInfo{{}}, - expected: true, - }, - { - message: "Close ref set in PR body", - prInfo: prBodyInfo{issueCloseRefs: map[string]int{"#942": 1}}, - commitInfos: []commitInfo{{}}, - expected: true, - }, - { - message: "Inform ref set in PR body", - prInfo: prBodyInfo{issueInformRefs: map[string]int{"#294": 1}}, - commitInfos: []commitInfo{{}}, - expected: true, - }, - { - message: "Epic set in commit", - prInfo: prBodyInfo{}, - commitInfos: []commitInfo{{sha: "128e2038", epicRefs: map[string]int{"CRDB-3919": 1}}}, - expected: true, - }, - { - message: "Epic None set in commit", - prInfo: prBodyInfo{}, - commitInfos: []commitInfo{{sha: "128e2038", epicNone: true}}, - expected: true, - }, - { - message: "Inform refs set in commit", - prInfo: prBodyInfo{}, - commitInfos: []commitInfo{{sha: "128e2038", issueInformRefs: map[string]int{"#491": 1}}}, - expected: true, - }, - { - message: "Close refs set in commit", - prInfo: prBodyInfo{}, - commitInfos: []commitInfo{{sha: "128e2038", issueCloseRefs: map[string]int{"#94712": 1}}}, - expected: true, - }, - } - - params := parameters{Repo: "cockroach", Org: "cockroachdb"} - - for _, tc := range testCases { - t.Run(tc.message, func(t *testing.T) { - result := checkForMissingRefs(tc.prInfo, tc.commitInfos, ¶ms) - assert.Equal(t, tc.expected, result) - }) - } -} - -func TestCheckPrEpicsUsedInCommits(t *testing.T) { - testCases := []struct { - message string - prInfo prBodyInfo - commitInfos []commitInfo - expected bool - }{ - // Failing checks - { - message: "Only one epic used in a commit", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-4123": 1, "CRDB-1929": 1}}, - commitInfos: []commitInfo{{epicRefs: map[string]int{"CRDB-4123": 1}}}, - expected: false, - }, - { - message: "An epic used in a commit is not used in the PR body", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-4123": 1, "CRDB-1929": 1}}, - commitInfos: []commitInfo{ - {epicRefs: map[string]int{"CRDB-4123": 1}}, - {epicRefs: map[string]int{"CRDB-1929": 1}}, - {epicRefs: map[string]int{"CRDB-94": 1}}, - }, - expected: false, - }, - { - message: "Commit without an epic", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-4123": 1, "CRDB-1929": 1}}, - commitInfos: []commitInfo{ - {epicRefs: map[string]int{"CRDB-4123": 1}}, - {epicRefs: map[string]int{"CRDB-1929": 1}}, - {}, - }, - expected: false, - }, - // Succeeding checks - { - message: "No epics in PR body", - prInfo: prBodyInfo{epicRefs: map[string]int{}}, - commitInfos: []commitInfo{{sha: "d53a4c0"}}, - expected: true, - }, - { - message: "One epic in PR body", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-48190": 1}}, - commitInfos: []commitInfo{{sha: "d53a4c0"}}, - expected: true, - }, - { - message: "Epics used in the same commit", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-4123": 1, "CRDB-1929": 1}}, - commitInfos: []commitInfo{{epicRefs: map[string]int{"CRDB-4123": 1, "CRDB-1929": 1}}}, - expected: true, - }, - { - message: "Epics used in different commits", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-4123": 1, "CRDB-1929": 1}}, - commitInfos: []commitInfo{ - {epicRefs: map[string]int{"CRDB-4123": 1}}, - {epicRefs: map[string]int{"CRDB-1929": 1}}, - }, - expected: true, - }, - { - message: "Epics used in multiple different commits", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-4123": 1, "CRDB-1929": 1}}, - commitInfos: []commitInfo{ - {epicRefs: map[string]int{"CRDB-4123": 1}}, - {epicRefs: map[string]int{"CRDB-1929": 1}}, - {epicRefs: map[string]int{"CRDB-1929": 1}}, - }, - expected: true, - }, - { - message: "Epics used in different commits plus an Epic none used", - prInfo: prBodyInfo{epicRefs: map[string]int{"CRDB-4123": 1, "CRDB-1929": 1}}, - commitInfos: []commitInfo{ - {epicRefs: map[string]int{"CRDB-4123": 1}}, - {epicRefs: map[string]int{"CRDB-1929": 1}}, - {epicNone: true}, - }, - expected: true, - }, - } - - params := parameters{Repo: "cockroach", Org: "cockroachdb"} - - for _, tc := range testCases { - t.Run(tc.message, func(t *testing.T) { - result := checkPrEpicsUsedInCommits(tc.prInfo, tc.commitInfos, ¶ms) - assert.Equal(t, tc.expected, result) - }) - } -} - -func TestCheckMultipleEpicsInCommits(t *testing.T) { - testCases := []struct { - message string - commitInfos []commitInfo - expected bool - }{ - { - message: "No epic used in a commit", - commitInfos: []commitInfo{{epicRefs: map[string]int{}}}, - expected: true, - }, - { - message: "One epic used in a commit", - commitInfos: []commitInfo{{epicRefs: map[string]int{"CRDB-4123": 1}}}, - expected: true, - }, - { - message: "Two epics used in a commit", - commitInfos: []commitInfo{{epicRefs: map[string]int{"CRDB-4123": 1, "CC-4589": 1}}}, - expected: false, - }, - { - message: "Multiple commits", - commitInfos: []commitInfo{ - {epicRefs: map[string]int{"CRDB-4123": 1}}, - {epicRefs: map[string]int{"CRDB-1929": 1}}, - {epicRefs: map[string]int{}}, - }, - expected: true, - }, - } - - params := parameters{Repo: "cockroach", Org: "cockroachdb"} - - for _, tc := range testCases { - t.Run(tc.message, func(t *testing.T) { - result := checkMultipleEpicsInCommits(tc.commitInfos, ¶ms) - assert.Equal(t, tc.expected, result) - }) - } -} - -func TestMultipPrBackport(t *testing.T) { - testCases := []struct { - message string - expected bool - }{ - { - message: "Backport 1/1 commits from #71039.", - expected: false, - }, - { - message: "Backport 2/2 commits from #79612", - expected: false, - }, - { - message: "Something else entirely", - expected: false, - }, - { - message: "Backport for 2 commits, 1/1 from #76516 and 1/1 from #79157.", - expected: true, - }, - { - message: `Backport: - * 1/1 commits from "sql: do not collect stats for virtual columns" (#68312) - * 1/1 commits from "sql: do not collect statistics on virtual columns" (#71105)`, - expected: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.message, func(t *testing.T) { - result := isMultiPrBackport(tc.message) - assert.Equal(t, tc.expected, result) - }) - } -} diff --git a/pkg/cmd/lint-epic-issue-refs/main.go b/pkg/cmd/lint-epic-issue-refs/main.go deleted file mode 100644 index 6d43f74f81c5..000000000000 --- a/pkg/cmd/lint-epic-issue-refs/main.go +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright 2022 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -// Check that GitHub PR descriptions and commit messages contain the -// expected epic and issue references. -package main - -import ( - "errors" - "fmt" - "os" - "strconv" - - "github.com/spf13/cobra" -) - -var rootCmd = &cobra.Command{ - Use: "lint-epic-issue-refs PR_NUMBER", - Short: "Check the body and commit messages of the indicated PR for epic and issue refs", - Args: cobra.ExactArgs(1), - Run: func(_ *cobra.Command, args []string) { - params := defaultEnvParameters() - prNumber, err := parseArgs(args) - if err != nil { - fmt.Printf("%v\n", err) - os.Exit(1) - } - - params.PrNumber = prNumber - err = lintPR(params) - if err != nil { - fmt.Printf("%v\n", err) - os.Exit(1) - } - }, -} - -func main() { - if err := rootCmd.Execute(); err != nil { - os.Exit(1) - } -} - -func parseArgs(args []string) (int, error) { - if len(args) < 1 { - return 0, errors.New("no PR number specified") - } - if len(args) > 1 { - return 0, errors.New("one argument is required: a PR number") - } - prNumber, err := strconv.Atoi(args[0]) - if err != nil { - return 0, fmt.Errorf("invalid PR number: %w", err) - } - - return prNumber, nil -} - -type parameters struct { - Token string // GitHub API token - Org string - Repo string - PrNumber int -} - -func defaultEnvParameters() *parameters { - const ( - githubTokenEnv = "GITHUB_TOKEN" - githubOrgEnv = "GITHUB_ORG" - githubRepoEnv = "GITHUB_REPO" - ) - - return ¶meters{ - Token: maybeEnv(githubTokenEnv, ""), - Org: maybeEnv(githubOrgEnv, "cockroachdb"), - Repo: maybeEnv(githubRepoEnv, "cockroach"), - } -} - -func maybeEnv(envKey, defaultValue string) string { - v := os.Getenv(envKey) - if v == "" { - return defaultValue - } - return v -}