Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ Add CODEOWNERS branch protection check #2057

Merged
merged 7 commits into from
Aug 29, 2022
Merged

✨ Add CODEOWNERS branch protection check #2057

merged 7 commits into from
Aug 29, 2022

Conversation

raghavkaul
Copy link
Contributor

What kind of change does this PR introduce?

Enhance Branch-Protection check to require CODEOWNER review.

This adds 'bonus points' to the Branch-Protection check, but doesn't detract from the score if it's not enabled.

What is the current behavior?

Today, the Branch-Protection check requires that code review must be completed before merge. In GitHub, CODEOWNERS can be assigned to subpaths in a. Without CODEOWNER review, anyone with write access to the repo can approve any PR. In large repos with many modules and many maintainers, CODEOWNERS allows maintainers to delegate code reviews and limit the scope of approval.

What is the new behavior (if this is a feature change)?**

Added a unit test; updated existing unit tests.

Does this PR introduce a user-facing change?

Scorecard will evaluate whether CODEOWNER review is required for protected branches.

@codecov
Copy link

codecov bot commented Jul 14, 2022 •
edited
Loading

Codecov Report

Merging #2057 (9efddd5) into main (6fc08e7) will increase coverage by 2.85%.
The diff coverage is 92.50%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2057      +/-   ##
==========================================
+ Coverage   41.87%   44.72%   +2.85%     
==========================================
  Files          95       95              
  Lines        7945     7977      +32     
==========================================
+ Hits         3327     3568     +241     
+ Misses       4358     4140     -218     
- Partials      260      269       +9

@raghavkaul raghavkaul temporarily deployed to integration-test July 14, 2022 20:41 Inactive
@github-actions
Copy link

Integration tests success for
[6a16492]
(https://github.com/ossf/scorecard/actions/runs/2673017055)

@raghavkaul raghavkaul marked this pull request as ready for review July 14, 2022 21:08
laurentsimon
laurentsimon reviewed Jul 15, 2022
View reviewed changes
Copy link
Contributor

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTm, looks great.
I think we just need to try to put the changes as part of an existing "level", see my comment.

Wdut?

checks/evaluation/branch_protection.go Outdated Show resolved Hide resolved
checks/evaluation/branch_protection.go Show resolved Hide resolved
@raghavkaul raghavkaul temporarily deployed to integration-test July 18, 2022 19:28 Inactive
laurentsimon
laurentsimon approved these changes Jul 18, 2022
View reviewed changes
Copy link
Contributor

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@raghavkaul raghavkaul temporarily deployed to integration-test July 18, 2022 19:41 Inactive
@github-actions
Copy link

Integration tests success for
[9610e62]
(https://github.com/ossf/scorecard/actions/runs/2693035239)

@github-actions
Copy link

Integration tests success for
[245f3b7]
(https://github.com/ossf/scorecard/actions/runs/2692949564)

@Spikebady
Copy link

Spikebady commented Jul 18, 2022 via email

@raghavkaul raghavkaul temporarily deployed to integration-test July 18, 2022 21:32 Inactive
@github-actions
Copy link

Integration tests success for
[f98c399]
(https://github.com/ossf/scorecard/actions/runs/2693612026)

@raghavkaul raghavkaul temporarily deployed to integration-test July 20, 2022 14:39 Inactive
@github-actions
Copy link

Integration tests success for
[377be84]
(https://github.com/ossf/scorecard/actions/runs/2705520493)

@laurentsimon laurentsimon enabled auto-merge (squash) July 20, 2022 15:10
olivekl
olivekl reviewed Jul 20, 2022
View reviewed changes
Copy link
Contributor

@olivekl olivekl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs look good to me, with a minor nit for formatting.

docs/checks/internal/checks.yaml Show resolved Hide resolved
@naveensrinivasan naveensrinivasan force-pushed the feature-codeowners-branch-protection branch from 377be84 to c14366b Compare July 27, 2022 17:46
@naveensrinivasan naveensrinivasan temporarily deployed to integration-test July 27, 2022 17:53 Inactive
@github-actions
Copy link

Integration tests success for
[c14366b]
(https://github.com/ossf/scorecard/actions/runs/2748573435)

@naveensrinivasan naveensrinivasan force-pushed the feature-codeowners-branch-protection branch from c14366b to 9b6bbca Compare July 27, 2022 19:42
@naveensrinivasan naveensrinivasan temporarily deployed to integration-test July 27, 2022 19:43 Inactive
@github-actions
Copy link

Integration tests success for
[9b6bbca]
(https://github.com/ossf/scorecard/actions/runs/2749214721)

@naveensrinivasan naveensrinivasan force-pushed the feature-codeowners-branch-protection branch from 9b6bbca to e223952 Compare July 31, 2022 13:11
@naveensrinivasan naveensrinivasan temporarily deployed to integration-test July 31, 2022 13:11 Inactive
@github-actions
Copy link

Integration tests success for
[e223952]
(https://github.com/ossf/scorecard/actions/runs/2769592302)

@naveensrinivasan
Copy link
Member

@raghavkaul The linter check is failing. Could you please fix that? We should be able to merge after that.

@github-actions
Copy link

Stale pull request message

@naveensrinivasan
Copy link
Member

@raghavkaul A friendly ping!

@github-actions
Copy link

Stale pull request message

auto-merge was automatically disabled August 29, 2022 17:14

Head branch was pushed to by a user without write access

@raghavkaul raghavkaul force-pushed the feature-codeowners-branch-protection branch from e223952 to 8aa0938 Compare August 29, 2022 17:14
@raghavkaul raghavkaul temporarily deployed to integration-test August 29, 2022 17:14 Inactive
@raghavkaul raghavkaul temporarily deployed to integration-test August 29, 2022 17:15 Inactive
@raghavkaul raghavkaul force-pushed the feature-codeowners-branch-protection branch from 4ea0772 to 9efddd5 Compare August 29, 2022 17:23
@raghavkaul raghavkaul temporarily deployed to integration-test August 29, 2022 17:27 Inactive
@github-actions
Copy link

Integration tests success for
[8aa0938]
(https://github.com/ossf/scorecard/actions/runs/2950391992)

@github-actions
Copy link

Integration tests success for
[4ea0772]
(https://github.com/ossf/scorecard/actions/runs/2950398797)

@github-actions
Copy link

Integration tests success for
[9efddd5]
(https://github.com/ossf/scorecard/actions/runs/2950444535)

@naveensrinivasan naveensrinivasan merged commit 621449f into ossf:main Aug 29, 2022
@raghavkaul raghavkaul deleted the feature-codeowners-branch-protection branch August 29, 2022 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurentsimon laurentsimon laurentsimon approved these changes

@olivekl olivekl olivekl approved these changes

@azeemshaikh38 azeemshaikh38 Awaiting requested review from azeemshaikh38

@justaugustus justaugustus Awaiting requested review from justaugustus

@naveensrinivasan naveensrinivasan Awaiting requested review from naveensrinivasan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@raghavkaul @Spikebady @naveensrinivasan @laurentsimon @olivekl