-
Notifications
You must be signed in to change notification settings - Fork 370
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(V2 Clients): Client refactor #1470
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1470 +/- ##
==========================================
- Coverage 67.33% 67.02% -0.32%
==========================================
Files 194 196 +2
Lines 18361 18478 +117
==========================================
+ Hits 12363 12384 +21
- Misses 5327 5416 +89
- Partials 671 678 +7 ☔ 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.
Generally LGTM, just one question regarding the file structure: shall we put implementation the same folder as the interface?
when we have more interfaces and implementation in the future, it will be tricky to track which interface an implementation implements.
I'm thinking that the interfaces will be moved to osv-scalibr at some point, so we shouldn't couple them in the same package. They are already under the same parent directory of clients, I think that's probably as close as I can get them? |
Paging impl has been extracted and moved to the matcher rather than the client. |
Second step of the client refactor after #1464
Changes:
Commented out Local client in resolve package to be completed in the followup.
Followup: Decide and implement the VulnClients in resolution.