-
Notifications
You must be signed in to change notification settings - Fork 596
🐛 Fix branch-protection ruleset handling when there are no include patterns #4835
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4835 +/- ##
==========================================
+ Coverage 66.80% 69.49% +2.68%
==========================================
Files 230 250 +20
Lines 16602 15595 -1007
==========================================
- Hits 11091 10837 -254
+ Misses 4808 3891 -917
- Partials 703 867 +164 🚀 New features to boost your workflow:
|
Are you sure about that? This is what I see on a test repo when making a ruleset without a target:
|
42b69d9 to
80630b8
Compare
ah, good point, fixed: 677aec3 |
Signed-off-by: Trask Stalnaker <[email protected]>
Signed-off-by: Trask Stalnaker <[email protected]>
80630b8 to
677aec3
Compare
|
Is this intentionally a draft PR? |
|
yeah, sorry, was going to confirm if we really needed it in OpenTelemetry or not (I think we may need to restructure our rulesets unrelated to this issue). do you prefer that I close until then or would you prefer I just mark it ready for review? |
|
Up to you, I mainly wanted to make sure this PR wasn't waiting for a review which wasn't assigned. But this fixes an edge case I didn't know about, so happy to merge it regardless of if OpenTelemetry ends up needing it. |
|
/scdiff generate Branch-Protection |
What kind of change does this PR introduce?
Bug fix – repository rulesets without any include patterns should apply to all branches which aren't explicitly excluded
What is the current behavior?
Scorecard ignores GitHub rulesets that rely on an empty
includelist (apply to all refs unless excluded), so branches covered only by those rulesets are reported as lacking protection. The branch-protection check emits false warnings such as “Warn: branch protection not enabled for branch 'xyz'”.What is the new behavior (if this is a feature change)?
Rulesets with no explicit include patterns are now treated as applying to every ref except those explicitly excluded, matching GitHub’s semantics. Branches governed by such rulesets are marked protected and no longer generate false warnings.
Which issue(s) this PR fixes
None
Special notes for your reviewer
None
Does this PR introduce a user-facing change?