🌱 Code Review: treat merging a PR as code review

checks/evaluation/code_review.go

@@ -61,12 +61,8 @@ func reviewScoreForChangeset(changeset *checker.Changeset) (score reviewScore) {
 		return reviewedOutsideGithub
 	}
 
-	for i := range changeset.Commits {
-		for _, review := range changeset.Commits[i].AssociatedMergeRequest.Reviews {
-			if review.State == "APPROVED" {
-				return changesReviewed
-			}
-		}
+	if changeset.ReviewPlatform == checker.ReviewPlatformGitHub {
+		return changesReviewed
 	}
 
 	return noReview

checks/evaluation/code_review_test.go

@@ -104,6 +104,24 @@ func TestCodeReview(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "implicit maintainer approval through github merge",
+			expected: scut.TestReturn{
+				Score: checker.MaxResultScore,
+			},
+			rawData: &checker.CodeReviewData{
+				DefaultBranchChangesets: []checker.Changeset{
+					{
+						ReviewPlatform: checker.ReviewPlatformGitHub,
+						Commits:        []clients.Commit{{SHA: "1"}},
+					},
+					{
+						ReviewPlatform: checker.ReviewPlatformGitHub,
+						Commits:        []clients.Commit{{SHA: "2"}},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {

checks/raw/code_review.go

@@ -143,6 +143,9 @@ func getChangesets(commits []clients.Commit) []checker.Changeset {
 
 	for i := range commits {
 		rev := detectCommitRevisionInfo(&commits[i])
+		if rev.ID == "" {
+			rev.ID = commits[i].SHA
+		}
 
 		if changeset, ok := changesetsByRevInfo[rev]; !ok {
 			newChangeset := checker.Changeset{

e2e/code_review_test.go

@@ -16,19 +16,20 @@ package e2e
 
 import (
 	"context"
+	"fmt"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/checks"
+	"github.com/ossf/scorecard/v4/checks/raw"
 	"github.com/ossf/scorecard/v4/clients"
 	"github.com/ossf/scorecard/v4/clients/githubrepo"
 	scut "github.com/ossf/scorecard/v4/utests"
 )
 
 // TODO: use dedicated repo that don't change.
-// TODO: need negative results.
 var _ = Describe("E2E TEST:"+checks.CheckCodeReview, func() {
 	Context("E2E TEST:Validating use of code reviews", func() {
 		It("Should return use of code reviews", func() {
@@ -81,5 +82,27 @@ var _ = Describe("E2E TEST:"+checks.CheckCodeReview, func() {
 			Expect(scut.ValidateTestReturn(nil, "use code reviews", &expected, &result, &dl)).Should(BeTrue())
 			Expect(repoClient.Close()).Should(BeNil())
 		})
+		It("Should return use of implicit code reviews at commit", func() {
+			repo, err := githubrepo.MakeGithubRepo("spring-projects/spring-framework")
+			Expect(err).Should(BeNil())
+			repoClient := githubrepo.CreateGithubRepoClient(context.Background(), logger)
+			err = repoClient.InitRepo(repo, "ca5e453f87f7e84033bb90a2fb54ee9f7fc94d61")
+			Expect(err).Should(BeNil())
+
+			reviewData, err := raw.CodeReview(repoClient)
+			Expect(err).Should(BeNil())
+			Expect(reviewData.DefaultBranchChangesets).ShouldNot(BeEmpty())
+
+			gh := 0
+			for _, cs := range reviewData.DefaultBranchChangesets {
+				if cs.ReviewPlatform == checker.ReviewPlatformGitHub {
+					fmt.Printf("found github revision %s in spring-framework", cs.RevisionID)
+					gh += 1
+				}
+			}
+			Expect(gh).Should(BeNumerically("==", 2))
+
+			Expect(repoClient.Close()).Should(BeNil())
+		})
 	})
 })

pkg/json_raw_results.go

@@ -84,7 +84,7 @@ type jsonUser struct {
 	Organizations []jsonOrganization `json:"organization,omitempty"`
 	// Companies refer to a claim by a user in their profile.
 	Companies        []jsonCompany `json:"company,omitempty"`
-	NumContributions int           `json:"NumContributions"`
+	NumContributions int           `json:"NumContributions,omitempty"`
 }
 
 type jsonContributors struct {
@@ -527,10 +527,42 @@ func (r *jsonScorecardRawResult) setDefaultCommitData(changesets []checker.Chang
 			})
 		}
 
+		reviews := []jsonReview{}
+		for j := range cs.Commits {
+			mr := cs.Commits[j].AssociatedMergeRequest
+			if mr.Reviews == nil {
+				continue
+			}
+			for k := range mr.Reviews {
+				r := mr.Reviews[k]
+				reviews = append(reviews, jsonReview{
+					State: r.State,
+					Reviewer: jsonUser{
+						Login: r.Author.Login,
+					},
+				})
+			}
+		}
+
+		// Only add the Merge Request opener as the PR author
+		authors := []jsonUser{}
+		for j := range cs.Commits {
+			mr := cs.Commits[j].AssociatedMergeRequest
+			if mr.Author.Login != "" {
+				authors = append(authors, jsonUser{
+					Login: mr.Author.Login,
+				})
+				break
+			}
+		}
+
 		r.Results.DefaultBranchChangesets = append(r.Results.DefaultBranchChangesets,
 			jsonDefaultBranchChangeset{
-				RevisionID: cs.RevisionID,
-				Commits:    commits,
+				RevisionID:     cs.RevisionID,
+				ReviewPlatform: cs.ReviewPlatform,
+				Commits:        commits,
+				Reviews:        reviews,
+				Authors:        authors,
 			},
 		)
 	}