From 4581c363cff785f6381b1ccf56e6a7abde373715 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Wed, 2 Feb 2022 16:01:35 -0800 Subject: [PATCH] Remove ListMergedPRs API (#1566) Co-authored-by: Azeem Shaikh --- checker/raw_result.go | 13 ++- checks/ci_tests.go | 10 +- checks/code_review_test.go | 138 +++++++++----------------- checks/evaluation/code_review.go | 4 +- checks/maintained.go | 4 +- checks/raw/code_review.go | 56 ++--------- checks/sast.go | 9 +- checks/sast_test.go | 93 ++++++++++++----- clients/commit.go | 9 +- clients/githubrepo/client.go | 5 - clients/githubrepo/graphql.go | 66 ++++-------- clients/localdir/client.go | 5 - clients/mockclients/repo_client.go | 15 --- clients/pull_request.go | 13 ++- clients/repo_client.go | 1 - docs/checks/internal/validate/main.go | 4 +- 16 files changed, 174 insertions(+), 271 deletions(-) diff --git a/checker/raw_result.go b/checker/raw_result.go index fd58dd7223e..8605af992f8 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -14,6 +14,8 @@ package checker +import "time" + // RawResults contains results before a policy // is applied. type RawResults struct { @@ -123,12 +125,13 @@ type DefaultBranchCommit struct { } // MergeRequest represents a merge request. -//nolint:govet +// nolint:govet type MergeRequest struct { - Number int - Labels []string - Reviews []Review - Author User + Number int + Labels []string + Reviews []Review + Author User + MergedAt time.Time } // Review represent a review using the built-in review system. diff --git a/checks/ci_tests.go b/checks/ci_tests.go index 09f39c79ae2..396f609d0ea 100644 --- a/checks/ci_tests.go +++ b/checks/ci_tests.go @@ -39,16 +39,18 @@ func init() { // CITests runs CI-Tests check. func CITests(c *checker.CheckRequest) checker.CheckResult { - prs, err := c.RepoClient.ListMergedPRs() + commits, err := c.RepoClient.ListCommits() if err != nil { - e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListMergedPRs: %v", err)) + e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListCommits: %v", err)) return checker.CreateRuntimeErrorResult(CheckCITests, e) } totalMerged := 0 totalTested := 0 - for index := range prs { - pr := &prs[index] + for i := range commits { + pr := &commits[i].AssociatedMergeRequest + // TODO(#575): We ignore associated PRs if Scorecard is being run on a fork + // but the PR was created in the original repo. if pr.MergedAt.IsZero() { continue } diff --git a/checks/code_review_test.go b/checks/code_review_test.go index b00bb8d1d53..e3da2ff5e02 100644 --- a/checks/code_review_test.go +++ b/checks/code_review_test.go @@ -37,7 +37,6 @@ func TestCodereview(t *testing.T) { name string commiterr error commits []clients.Commit - prs []clients.PullRequest expected checker.CheckResult }{ { @@ -70,29 +69,23 @@ func TestCodereview(t *testing.T) { }, { name: "Valid GitHub PR", - prs: []clients.PullRequest{ - { - Number: 1, - MergedAt: time.Now(), - Reviews: []clients.Review{ - { - State: "APPROVED", - }, - }, - MergeCommit: clients.Commit{ - SHA: "sha", - }, - }, - }, commits: []clients.Commit{ { SHA: "sha", Committer: clients.User{ Login: "user", }, + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + MergedAt: time.Now(), + Reviews: []clients.Review{ + { + State: "APPROVED", + }, + }, + }, }, }, - expected: checker.CheckResult{ Score: 10, Pass: true, @@ -100,29 +93,23 @@ func TestCodereview(t *testing.T) { }, { name: "Valid Prow PR as not a bot", - prs: []clients.PullRequest{ - { - Number: 1, - MergedAt: time.Now(), - Labels: []clients.Label{ - { - Name: "lgtm", - }, - }, - MergeCommit: clients.Commit{ - SHA: "sha", - }, - }, - }, commits: []clients.Commit{ { SHA: "sha", Committer: clients.User{ Login: "user", }, + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + MergedAt: time.Now(), + Labels: []clients.Label{ + { + Name: "lgtm", + }, + }, + }, }, }, - expected: checker.CheckResult{ Score: 10, Pass: true, @@ -130,29 +117,23 @@ func TestCodereview(t *testing.T) { }, { name: "Valid Prow PR and commits as bot", - prs: []clients.PullRequest{ - { - Number: 1, - MergedAt: time.Now(), - Labels: []clients.Label{ - { - Name: "lgtm", - }, - }, - MergeCommit: clients.Commit{ - SHA: "sha", - }, - }, - }, commits: []clients.Commit{ { SHA: "sha", Committer: clients.User{ Login: "bot", }, + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + MergedAt: time.Now(), + Labels: []clients.Label{ + { + Name: "lgtm", + }, + }, + }, }, }, - expected: checker.CheckResult{ Score: 10, Pass: true, @@ -160,60 +141,43 @@ func TestCodereview(t *testing.T) { }, { name: "Valid PR's and commits with merged by someone else", - prs: []clients.PullRequest{ - { - Number: 1, - Labels: []clients.Label{ - { - Name: "lgtm", - }, - }, - MergeCommit: clients.Commit{ - SHA: "sha", - Committer: clients.User{ - Login: "bot", - }, - }, - }, - }, commits: []clients.Commit{ { SHA: "sha", Committer: clients.User{ Login: "bob", }, + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + Labels: []clients.Label{ + { + Name: "lgtm", + }, + }, + }, }, }, - expected: checker.CheckResult{ Score: 0, }, }, { name: "2 PRs 2 review on GitHub", - prs: []clients.PullRequest{ - { - Number: 1, - MergedAt: time.Now(), - Reviews: []clients.Review{ - { - State: "APPROVED", - }, - }, - MergeCommit: clients.Commit{ - SHA: "sha", - Committer: clients.User{ - Login: "bot", - }, - }, - }, - }, commits: []clients.Commit{ { SHA: "sha", Committer: clients.User{ Login: "bob", }, + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + MergedAt: time.Now(), + Reviews: []clients.Review{ + { + State: "APPROVED", + }, + }, + }, }, { SHA: "sha2", @@ -222,7 +186,6 @@ func TestCodereview(t *testing.T) { }, }, }, - expected: checker.CheckResult{ Score: 5, }, @@ -235,18 +198,7 @@ func TestCodereview(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) mockRepo := mockrepo.NewMockRepoClient(ctrl) - mockRepo.EXPECT().ListMergedPRs().DoAndReturn(func() ([]clients.PullRequest, error) { - if tt.err != nil { - return tt.prs, tt.err - } - return tt.prs, tt.err - }).AnyTimes() - mockRepo.EXPECT().ListCommits().DoAndReturn(func() ([]clients.Commit, error) { - if tt.commiterr != nil { - return tt.commits, tt.commiterr - } - return tt.commits, tt.err - }).AnyTimes() + mockRepo.EXPECT().ListCommits().Return(tt.commits, tt.err).AnyTimes() req := checker.CheckRequest{ RepoClient: mockRepo, diff --git a/checks/evaluation/code_review.go b/checks/evaluation/code_review.go index 7514eb43e1e..9a48a6fde51 100644 --- a/checks/evaluation/code_review.go +++ b/checks/evaluation/code_review.go @@ -126,7 +126,7 @@ func getApprovedReviewSystem(c *checker.DefaultBranchCommit, dl checker.DetailLo func isReviewedOnGitHub(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool { mr := c.MergeRequest - if mr == nil { + if mr == nil || mr.MergedAt.IsZero() { return false } @@ -163,7 +163,7 @@ func isReviewedOnProw(c *checker.DefaultBranchCommit, dl checker.DetailLogger) b return true } - if c.MergeRequest != nil { + if c.MergeRequest != nil && !c.MergeRequest.MergedAt.IsZero() { for _, l := range c.MergeRequest.Labels { if l == "lgtm" || l == "approved" { dl.Debug3(&checker.LogMessage{ diff --git a/checks/maintained.go b/checks/maintained.go index b4d7089af5e..7e33a962ad1 100644 --- a/checks/maintained.go +++ b/checks/maintained.go @@ -59,8 +59,8 @@ func IsMaintained(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckMaintained, e) } commitsWithinThreshold := 0 - for _, commit := range commits { - if commit.CommittedDate.After(threshold) { + for i := range commits { + if commits[i].CommittedDate.After(threshold) { commitsWithinThreshold++ } } diff --git a/checks/raw/code_review.go b/checks/raw/code_review.go index 07f88e3d984..8eb6453f0c3 100644 --- a/checks/raw/code_review.go +++ b/checks/raw/code_review.go @@ -31,63 +31,21 @@ func CodeReview(c clients.RepoClient) (checker.CodeReviewData, error) { return checker.CodeReviewData{}, fmt.Errorf("%w", err) } - oc := make(map[string]checker.DefaultBranchCommit) - for _, commit := range commits { - com := commitRequest(commit) - // Keep an index of commits by SHA. - oc[commit.SHA] = com - } - - // Look at merge requests. - mrs, err := c.ListMergedPRs() - if err != nil { - return checker.CodeReviewData{}, fmt.Errorf("%w", err) - } - - for i := range mrs { - mr := mrs[i] - - if mr.MergedAt.IsZero() { - continue - } - - // If the merge request is not a recent commit, skip. - com, exists := oc[mr.MergeCommit.SHA] - - if exists { - // Sanity checks the logins are the same. - // TODO(#1543): re-enable this code. - //nolint:gocritic - /*if com.Committer.Login != mr.MergeCommit.Committer.Login { - return checker.CodeReviewData{}, sce.WithMessage(sce.ErrScorecardInternal, - fmt.Sprintf("commit login (%s) different from merge request commit login (%s)", - com.Committer.Login, mr.MergeCommit.Committer.Login)) - }*/ - - // We have a recent merge request, update it. - com.MergeRequest = mergeRequest(&mr) - oc[com.SHA] = com - } - } - - for _, v := range oc { - results = append(results, v) - } - - if len(results) > 0 { - return checker.CodeReviewData{DefaultBranchCommits: results}, nil + for i := range commits { + results = append(results, getRawDataFrom(&commits[i])) } return checker.CodeReviewData{DefaultBranchCommits: results}, nil } -func commitRequest(c clients.Commit) checker.DefaultBranchCommit { +func getRawDataFrom(c *clients.Commit) checker.DefaultBranchCommit { r := checker.DefaultBranchCommit{ Committer: checker.User{ Login: c.Committer.Login, }, SHA: c.SHA, CommitMessage: c.Message, + MergeRequest: mergeRequest(&c.AssociatedMergeRequest), } return r @@ -99,9 +57,9 @@ func mergeRequest(mr *clients.PullRequest) *checker.MergeRequest { Author: checker.User{ Login: mr.Author.Login, }, - - Labels: labels(mr), - Reviews: reviews(mr), + MergedAt: mr.MergedAt, + Labels: labels(mr), + Reviews: reviews(mr), } return &r } diff --git a/checks/sast.go b/checks/sast.go index 30f39cc1708..b9f311ebc4e 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -108,16 +108,19 @@ func SAST(c *checker.CheckRequest) checker.CheckResult { // nolint func sastToolInCheckRuns(c *checker.CheckRequest) (int, error) { - prs, err := c.RepoClient.ListMergedPRs() + commits, err := c.RepoClient.ListCommits() if err != nil { //nolint return checker.InconclusiveResultScore, - sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListMergedPRs: %v", err)) + sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListCommits: %v", err)) } totalMerged := 0 totalTested := 0 - for _, pr := range prs { + for _, commit := range commits { + pr := commit.AssociatedMergeRequest + // TODO(#575): We ignore associated PRs if Scorecard is being run on a fork + // but the PR was created in the original repo. if pr.MergedAt.IsZero() { continue } diff --git a/checks/sast_test.go b/checks/sast_test.go index d29b873b93c..4f0ce30b49c 100644 --- a/checks/sast_test.go +++ b/checks/sast_test.go @@ -30,10 +30,11 @@ import ( func TestSAST(t *testing.T) { t.Parallel() - //nolint + + // nolint: govet, goerr113 tests := []struct { name string - prs []clients.PullRequest + commits []clients.Commit err error searchresult clients.SearchResponse checkRuns []clients.CheckRun @@ -42,23 +43,25 @@ func TestSAST(t *testing.T) { }{ { name: "SAST checker should return failed status when no PRs are found", - prs: []clients.PullRequest{}, + commits: []clients.Commit{}, searchresult: clients.SearchResponse{}, checkRuns: []clients.CheckRun{}, }, { name: "SAST checker should return failed status when no PRs are found", err: errors.New("error"), - prs: []clients.PullRequest{}, + commits: []clients.Commit{}, searchresult: clients.SearchResponse{}, checkRuns: []clients.CheckRun{}, expected: checker.CheckResult{Score: -1, Pass: false}, }, { name: "Successful SAST checker should return success status", - prs: []clients.PullRequest{ + commits: []clients.Commit{ { - MergedAt: time.Now().Add(time.Hour - 1), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 1), + }, }, }, searchresult: clients.SearchResponse{}, @@ -78,18 +81,26 @@ func TestSAST(t *testing.T) { }, { name: "Failed SAST checker should return success status", - prs: []clients.PullRequest{ + commits: []clients.Commit{ { - MergedAt: time.Now().Add(time.Hour - 1), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 1), + }, }, { - MergedAt: time.Now().Add(time.Hour - 10), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 10), + }, }, { - MergedAt: time.Now().Add(time.Hour - 20), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 20), + }, }, { - MergedAt: time.Now().Add(time.Hour - 30), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 30), + }, }, }, searchresult: clients.SearchResponse{Hits: 1, Results: []clients.SearchResult{{ @@ -110,18 +121,26 @@ func TestSAST(t *testing.T) { }, { name: "Failed SAST checker with checkRuns not completed", - prs: []clients.PullRequest{ + commits: []clients.Commit{ { - MergedAt: time.Now().Add(time.Hour - 1), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 1), + }, }, { - MergedAt: time.Now().Add(time.Hour - 10), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 10), + }, }, { - MergedAt: time.Now().Add(time.Hour - 20), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 20), + }, }, { - MergedAt: time.Now().Add(time.Hour - 30), + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 30), + }, }, }, searchresult: clients.SearchResponse{}, @@ -139,9 +158,34 @@ func TestSAST(t *testing.T) { }, { name: "Failed SAST with PullRequest not merged", - prs: []clients.PullRequest{ + commits: []clients.Commit{ + { + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + }, + }, + }, + searchresult: clients.SearchResponse{}, + checkRuns: []clients.CheckRun{ { - Number: 1, + App: clients.CheckRunApp{ + Slug: "lgtm-com", + }, + }, + }, + expected: checker.CheckResult{ + Score: 0, + Pass: false, + }, + }, + { + name: "Merged PullRequest in a different repo", + commits: []clients.Commit{ + { + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now(), + Number: 1, + }, }, }, searchresult: clients.SearchResponse{}, @@ -167,20 +211,19 @@ func TestSAST(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) - mockRepo := mockrepo.NewMockRepoClient(ctrl) - - mockRepo.EXPECT().ListMergedPRs().DoAndReturn(func() ([]clients.PullRequest, error) { + mockRepoClient := mockrepo.NewMockRepoClient(ctrl) + mockRepoClient.EXPECT().ListCommits().DoAndReturn(func() ([]clients.Commit, error) { if tt.err != nil { return nil, tt.err } - return tt.prs, tt.err + return tt.commits, tt.err }) - mockRepo.EXPECT().ListCheckRunsForRef("").Return(tt.checkRuns, nil).AnyTimes() - mockRepo.EXPECT().Search(searchRequest).Return(tt.searchresult, nil).AnyTimes() + mockRepoClient.EXPECT().ListCheckRunsForRef("").Return(tt.checkRuns, nil).AnyTimes() + mockRepoClient.EXPECT().Search(searchRequest).Return(tt.searchresult, nil).AnyTimes() dl := scut.TestDetailLogger{} req := checker.CheckRequest{ - RepoClient: mockRepo, + RepoClient: mockRepoClient, Ctx: context.TODO(), Dlogger: &dl, } diff --git a/clients/commit.go b/clients/commit.go index fb8105cd0d3..3ff5cc13994 100644 --- a/clients/commit.go +++ b/clients/commit.go @@ -18,8 +18,9 @@ import "time" // Commit represents a Git commit. type Commit struct { - CommittedDate time.Time - Message string - SHA string - Committer User + CommittedDate time.Time + Message string + SHA string + Committer User + AssociatedMergeRequest PullRequest } diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index e21f3ca85f8..49a1b89ea5f 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -113,11 +113,6 @@ func (client *Client) GetFileContent(filename string) ([]byte, error) { return client.tarball.getFileContent(filename) } -// ListMergedPRs implements RepoClient.ListMergedPRs. -func (client *Client) ListMergedPRs() ([]clients.PullRequest, error) { - return client.graphClient.getMergedPRs() -} - // ListCommits implements RepoClient.ListCommits. func (client *Client) ListCommits() ([]clients.Commit, error) { return client.graphClient.getCommits() diff --git a/clients/githubrepo/graphql.go b/clients/githubrepo/graphql.go index 15b7de0ea31..ba868bdbf3e 100644 --- a/clients/githubrepo/graphql.go +++ b/clients/githubrepo/graphql.go @@ -122,7 +122,6 @@ type graphqlHandler struct { errSetup error owner string repo string - prs []clients.PullRequest commits []clients.Commit issues []clients.Issue archived bool @@ -154,8 +153,7 @@ func (handler *graphqlHandler) setup() error { return } handler.archived = bool(handler.data.Repository.IsArchived) - handler.prs = pullRequestsFrom(handler.data, handler.owner, handler.repo) - handler.commits, handler.errSetup = commitsFrom(handler.data) + handler.commits, handler.errSetup = commitsFrom(handler.data, handler.owner, handler.repo) if handler.errSetup != nil { return } @@ -164,13 +162,6 @@ func (handler *graphqlHandler) setup() error { return handler.errSetup } -func (handler *graphqlHandler) getMergedPRs() ([]clients.PullRequest, error) { - if err := handler.setup(); err != nil { - return nil, fmt.Errorf("error during graphqlHandler.setup: %w", err) - } - return handler.prs, nil -} - func (handler *graphqlHandler) getCommits() ([]clients.Commit, error) { if err := handler.setup(); err != nil { return nil, fmt.Errorf("error during graphqlHandler.setup: %w", err) @@ -192,67 +183,43 @@ func (handler *graphqlHandler) isArchived() (bool, error) { return handler.archived, nil } -func pullRequestsFrom(data *graphqlData, repoOwner, repoName string) []clients.PullRequest { - var ret []clients.PullRequest +// nolint: unparam +func commitsFrom(data *graphqlData, repoOwner, repoName string) ([]clients.Commit, error) { + ret := make([]clients.Commit, 0) for _, commit := range data.Repository.DefaultBranchRef.Target.Commit.History.Nodes { + var committer string + if commit.Committer.User.Login != nil { + committer = *commit.Committer.User.Login + } + // TODO(#1543): Figure out a way to safely get committer if `User.Login` is `nil`. + var associatedPR clients.PullRequest for i := range commit.AssociatedPullRequests.Nodes { pr := commit.AssociatedPullRequests.Nodes[i] if pr.MergeCommit.Oid != commit.Oid || - string(pr.Repository.Name) != repoName || - string(pr.Repository.Owner.Login) != repoOwner { + string(pr.Repository.Owner.Login) != repoOwner || + string(pr.Repository.Name) != repoName { continue } - toAppend := clients.PullRequest{ + associatedPR = clients.PullRequest{ Number: int(pr.Number), HeadSHA: string(pr.HeadRefOid), MergedAt: pr.MergedAt.Time, Author: clients.User{ Login: string(pr.Author.Login), }, - // TODO(#575): Use the original commit data here directly. - MergeCommit: clients.Commit{ - Committer: clients.User{ - Login: string(commit.Author.User.Login), - }, - SHA: string(pr.MergeCommit.Oid), - }, } - for _, label := range pr.Labels.Nodes { - toAppend.Labels = append(toAppend.Labels, clients.Label{ + associatedPR.Labels = append(associatedPR.Labels, clients.Label{ Name: string(label.Name), }) } for _, review := range pr.Reviews.Nodes { - r := clients.Review{ + associatedPR.Reviews = append(associatedPR.Reviews, clients.Review{ State: string(review.State), - } - - a := clients.User{ - Login: string(review.Author.Login), - } - if a.Login != "" { - r.Author = &a - } - toAppend.Reviews = append(toAppend.Reviews, r) + }) } - - ret = append(ret, toAppend) break } - } - return ret -} - -// nolint: unparam -func commitsFrom(data *graphqlData) ([]clients.Commit, error) { - ret := make([]clients.Commit, 0) - for _, commit := range data.Repository.DefaultBranchRef.Target.Commit.History.Nodes { - var committer string - if commit.Committer.User.Login != nil { - committer = *commit.Committer.User.Login - } - // TODO(#1543): Figure out a way to safely get committer if `User.Login` is `nil`. ret = append(ret, clients.Commit{ CommittedDate: commit.CommittedDate.Time, Message: string(commit.Message), @@ -260,6 +227,7 @@ func commitsFrom(data *graphqlData) ([]clients.Commit, error) { Committer: clients.User{ Login: committer, }, + AssociatedMergeRequest: associatedPR, }) } return ret, nil diff --git a/clients/localdir/client.go b/clients/localdir/client.go index 27e5c0f30e8..6f0e76d44f0 100644 --- a/clients/localdir/client.go +++ b/clients/localdir/client.go @@ -153,11 +153,6 @@ func (client *localDirClient) GetFileContent(filename string) ([]byte, error) { return getFileContent(client.path, filename) } -// ListMergedPRs implements RepoClient.ListMergedPRs. -func (client *localDirClient) ListMergedPRs() ([]clients.PullRequest, error) { - return nil, fmt.Errorf("ListMergedPRs: %w", clients.ErrUnsupportedFeature) -} - // ListBranches implements RepoClient.ListBranches. func (client *localDirClient) ListBranches() ([]*clients.BranchRef, error) { return nil, fmt.Errorf("ListBranches: %w", clients.ErrUnsupportedFeature) diff --git a/clients/mockclients/repo_client.go b/clients/mockclients/repo_client.go index c1ff27ff3f8..f6a70371ffe 100644 --- a/clients/mockclients/repo_client.go +++ b/clients/mockclients/repo_client.go @@ -212,21 +212,6 @@ func (mr *MockRepoClientMockRecorder) ListIssues() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListIssues", reflect.TypeOf((*MockRepoClient)(nil).ListIssues)) } -// ListMergedPRs mocks base method. -func (m *MockRepoClient) ListMergedPRs() ([]clients.PullRequest, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListMergedPRs") - ret0, _ := ret[0].([]clients.PullRequest) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ListMergedPRs indicates an expected call of ListMergedPRs. -func (mr *MockRepoClientMockRecorder) ListMergedPRs() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListMergedPRs", reflect.TypeOf((*MockRepoClient)(nil).ListMergedPRs)) -} - // ListReleases mocks base method. func (m *MockRepoClient) ListReleases() ([]clients.Release, error) { m.ctrl.T.Helper() diff --git a/clients/pull_request.go b/clients/pull_request.go index e4700f74098..491d752c263 100644 --- a/clients/pull_request.go +++ b/clients/pull_request.go @@ -21,13 +21,12 @@ import ( // PullRequest struct represents a PR as returned by RepoClient. // nolint: govet type PullRequest struct { - MergedAt time.Time - MergeCommit Commit - Number int - HeadSHA string - Labels []Label - Reviews []Review - Author User + Number int + MergedAt time.Time + HeadSHA string + Author User + Labels []Label + Reviews []Review } // Label represents a PR label. diff --git a/clients/repo_client.go b/clients/repo_client.go index 97dc59b07e5..5f78f46b017 100644 --- a/clients/repo_client.go +++ b/clients/repo_client.go @@ -27,7 +27,6 @@ type RepoClient interface { IsArchived() (bool, error) ListFiles(predicate func(string) (bool, error)) ([]string, error) GetFileContent(filename string) ([]byte, error) - ListMergedPRs() ([]PullRequest, error) ListBranches() ([]*BranchRef, error) GetDefaultBranch() (*BranchRef, error) ListCommits() ([]Commit, error) diff --git a/docs/checks/internal/validate/main.go b/docs/checks/internal/validate/main.go index 734c23943d4..2fd38d33594 100644 --- a/docs/checks/internal/validate/main.go +++ b/docs/checks/internal/validate/main.go @@ -40,7 +40,6 @@ var ( "IsArchived": {"GitHub"}, "ListFiles": {"GitHub", "local"}, "GetFileContent": {"GitHub", "local"}, - "ListMergedPRs": {"GitHub"}, "ListBranches": {"GitHub"}, "GetDefaultBranch": {"GitHub"}, "ListCommits": {"GitHub"}, @@ -67,7 +66,8 @@ func listCheckFiles() (map[string]string, error) { } for _, file := range files { - if !strings.HasSuffix(file.Name(), ".go") || file.IsDir() { + if !strings.HasSuffix(file.Name(), ".go") || + strings.HasSuffix(file.Name(), "_test.go") || file.IsDir() { continue }