-
Notifications
You must be signed in to change notification settings - Fork 585
🐛 Skip CODEOWNERS file in contributors check if there is a parsing error #4851
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
Signed-off-by: juanis2112 <[email protected]>
6941e8e to
7214d48
Compare
spencerschrock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I plan to open an issue in the hmarr/codeowners
package, as I believe they could benefit from supporting the ^ character in their parsing patterns
Is that a valid CODEOWNERS character? I noticed the upstream VLLM fixed their codeowners, so I suspect it was a temporary mistake.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4851 +/- ##
==========================================
+ Coverage 66.80% 69.59% +2.78%
==========================================
Files 230 251 +21
Lines 16602 15657 -945
==========================================
- Hits 11091 10896 -195
+ Misses 4808 3891 -917
- Partials 703 870 +167 🚀 New features to boost your workflow:
|
Signed-off-by: juanis2112 <[email protected]>
spencerschrock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great thanks
What kind of change does this PR introduce?
Bug Fix
What is the current behavior?
When running scorecard on the repo: https://github.com/IBM/vllm I run into the following error:
What is the new behavior (if this is a feature change)?**
The Codeowners file is skipped if there are parsing errors found so now running scorecard on the repo https://github.com/IBM/vllm shows a score instead of error.
Which issue(s) this PR fixes
Fixes #4768
Special notes for your reviewer
I plan to open an issue in the hmarr/codeowners
package, as I believe they could benefit from supporting the ^ character in their parsing patterns. Regardless, it’s better for Scorecards to skip the CODEOWNERS file if a parsing error occurs and still produce a score, rather than invalidating the entire check. This change addresses that need.
We could optionally print a message to inform the user that a CODEOWNERS file was skipped, happy to add that if it’s useful.
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.)