-
Notifications
You must be signed in to change notification settings - Fork 464
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
feat: Adding language parser for RPM file #3845
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3845 +/- ##
==========================================
+ Coverage 75.41% 80.70% +5.29%
==========================================
Files 808 810 +2
Lines 11983 12239 +256
Branches 1598 1657 +59
==========================================
+ Hits 9037 9878 +841
+ Misses 2593 1915 -678
- Partials 353 446 +93
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Thanks for looking into this! I need to go look at the original PR and see if we can merge it and then yours so that the original contribution team gets appropriate credit in git, and it's probably not going to happen for a few days. But in the meantime, here's some cleanup requests:
- please remove the commented out code from the PR. I don't mind having a bit of commented code around if it useful for debugging but I think you're probably done with the parts you have commented out here.
- Is it possible to replace test_scan_files_multiline with something that tests the multiline capability and actually works? If you're not up to doing it, can you file an issue saying it needs a replacement? It needs one even now since it's failing on windows.
- If nothing uses those condensed-download/*.rpm files, they should be removed in this PR alongside the test.
Nw, I like working on this project. |
Close #2916