Skip to content

Commit

Permalink
Merge pull request #77 from ossf/a8
Browse files Browse the repository at this point in the history
Improve code review check to account for diff author-committer usecase.
  • Loading branch information
inferno-chromium committed Nov 24, 2020
2 parents c3c5bc8 + 09518b4 commit 7a3e18e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
22 changes: 21 additions & 1 deletion checks/code_review.go
Expand Up @@ -55,17 +55,37 @@ func GithubCodeReview(c checker.Checker) checker.CheckResult {
continue
}
totalMerged++
// Merged PR!

// check if the PR is approved by a reviewer
foundApprovedReview := false
reviews, _, err := c.Client.PullRequests.ListReviews(c.Ctx, c.Owner, c.Repo, pr.GetNumber(), &github.ListOptions{})
if err != nil {
continue
}
for _, r := range reviews {
if r.GetState() == "APPROVED" {
c.Logf("found review approved pr: %d", pr.GetNumber())
totalReviewed++
foundApprovedReview = true
break
}
}

// check if the PR is committed by someone other than author. this is kind
// of equivalent to a review and is done several times on small prs to save
// time on clicking the approve button.
if !foundApprovedReview {
commit, _, err := c.Client.Repositories.GetCommit(c.Ctx, c.Owner, c.Repo, pr.GetMergeCommitSHA())
if err == nil {
commitAuthor := commit.GetAuthor().GetLogin()
commitCommitter := commit.GetCommitter().GetLogin()
if commitAuthor != "" && commitCommitter != "" && commitAuthor != commitCommitter {
c.Logf("found pr with committer different than author: %d", pr.GetNumber())
totalReviewed++
}
}
}

}

if totalReviewed > 0 {
Expand Down
7 changes: 7 additions & 0 deletions cmd/root.go
Expand Up @@ -113,6 +113,7 @@ type checkResult struct {
CheckName string
Pass bool
Confidence int
Details []string
}

type record struct {
Expand All @@ -127,11 +128,17 @@ func outputJSON(results []pkg.Result) {
Repo: repo.String(),
Date: d.Format("2006-01-02"),
}

for _, r := range results {
var details []string
if showDetails {
details = r.Cr.Details
}
or.Checks = append(or.Checks, checkResult{
CheckName: r.Name,
Pass: r.Cr.Pass,
Confidence: r.Cr.Confidence,
Details: details,
})
}
output, err := json.Marshal(or)
Expand Down

0 comments on commit 7a3e18e

Please sign in to comment.