mirror of
https://github.com/ossf/scorecard.git
synced 2024-11-04 03:52:31 +03:00
Remove ListMergedPRs API (#1566)
Co-authored-by: Azeem Shaikh <azeems@google.com>
This commit is contained in:
parent
9037444513
commit
4581c363cf
@ -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.
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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,
|
||||
|
@ -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{
|
||||
|
@ -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++
|
||||
}
|
||||
}
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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{
|
||||
{
|
||||
Number: 1,
|
||||
AssociatedMergeRequest: clients.PullRequest{
|
||||
Number: 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
searchresult: clients.SearchResponse{},
|
||||
checkRuns: []clients.CheckRun{
|
||||
{
|
||||
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,
|
||||
}
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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()
|
||||
|
@ -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,60 +183,8 @@ func (handler *graphqlHandler) isArchived() (bool, error) {
|
||||
return handler.archived, nil
|
||||
}
|
||||
|
||||
func pullRequestsFrom(data *graphqlData, repoOwner, repoName string) []clients.PullRequest {
|
||||
var ret []clients.PullRequest
|
||||
for _, commit := range data.Repository.DefaultBranchRef.Target.Commit.History.Nodes {
|
||||
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 {
|
||||
continue
|
||||
}
|
||||
toAppend := 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{
|
||||
Name: string(label.Name),
|
||||
})
|
||||
}
|
||||
for _, review := range pr.Reviews.Nodes {
|
||||
r := 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) {
|
||||
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
|
||||
@ -253,6 +192,34 @@ func commitsFrom(data *graphqlData) ([]clients.Commit, error) {
|
||||
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.Owner.Login) != repoOwner ||
|
||||
string(pr.Repository.Name) != repoName {
|
||||
continue
|
||||
}
|
||||
associatedPR = clients.PullRequest{
|
||||
Number: int(pr.Number),
|
||||
HeadSHA: string(pr.HeadRefOid),
|
||||
MergedAt: pr.MergedAt.Time,
|
||||
Author: clients.User{
|
||||
Login: string(pr.Author.Login),
|
||||
},
|
||||
}
|
||||
for _, label := range pr.Labels.Nodes {
|
||||
associatedPR.Labels = append(associatedPR.Labels, clients.Label{
|
||||
Name: string(label.Name),
|
||||
})
|
||||
}
|
||||
for _, review := range pr.Reviews.Nodes {
|
||||
associatedPR.Reviews = append(associatedPR.Reviews, clients.Review{
|
||||
State: string(review.State),
|
||||
})
|
||||
}
|
||||
break
|
||||
}
|
||||
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
|
||||
|
@ -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)
|
||||
|
@ -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()
|
||||
|
@ -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.
|
||||
|
@ -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)
|
||||
|
@ -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
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user