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.

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

🐛 Branch-Protection: use debug message when unsure if PRs are required #3917

Merged

Conversation

spencerschrock
Copy link
Member

@spencerschrock spencerschrock commented Mar 4, 2024

What kind of change does this PR introduce?

bug fix

What is the current behavior?

When run with a non-admin token, if we cant tell if PRs are required, we throw a warning.

Warn: could not determine whether branch 'main' requires PRs to change code

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

Since requiring PRs is listed as potentially admin only data, not having the data is now DEBUG, not WARN.

Debug: could not determine whether branch 'main' requires PRs to change code

scorecard/docs/checks.md

Lines 113 to 115 in 90a3708

Tier 2 Requirements (6/10 points):
- Require at least 1 reviewer for approval before merging (for administrators, this requirement weights twice than the others in this tier)
- For administrators: Require PRs prior to make any code changes

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

NONE

Special notes for your reviewer

I got rid of the switch statement since the function is only called by 3 probes, and all of them want that behavior.

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

warning when the data isn't available isn't intended.

Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock requested a review from a team as a code owner March 4, 2024 21:54
@spencerschrock spencerschrock requested review from naveensrinivasan and justaugustus and removed request for a team March 4, 2024 21:54
Copy link

codecov bot commented Mar 4, 2024

Codecov Report

Merging #3917 (a553664) into main (e74d90d) will increase coverage by 1.93%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3917      +/-   ##
==========================================
+ Coverage   66.67%   68.61%   +1.93%     
==========================================
  Files         234      234              
  Lines       15864    15859       -5     
==========================================
+ Hits        10577    10881     +304     
+ Misses       4633     4287     -346     
- Partials      654      691      +37     

@spencerschrock spencerschrock enabled auto-merge (squash) March 6, 2024 21:07
@spencerschrock spencerschrock merged commit 0a6b06a into ossf:main Mar 6, 2024
38 checks passed
@spencerschrock spencerschrock deleted the fix-branch-protection-pr-required branch March 6, 2024 21:10
fhoeborn pushed a commit to fhoeborn/scorecard that referenced this pull request Apr 1, 2024
ossf#3917)

warning when the data isn't available isn't intended.

Signed-off-by: Spencer Schrock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants