-
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
refactor: imodels PackageInfo refactor to use functions #1482
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1482 +/- ##
==========================================
- Coverage 66.91% 66.86% -0.06%
==========================================
Files 197 198 +1
Lines 18619 18650 +31
==========================================
+ Hits 12459 12470 +11
- Misses 5474 5496 +22
+ Partials 686 684 -2 ☔ 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.
nice!
|
||
AdditionalLocations []string // Contains Inventory.Locations[1..] | ||
if metadata, ok := pkg.Inventory.Metadata.(*dpkg.Metadata); ok { | ||
// Debian uses source name on osv.dev |
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.
Do we need to add a TODO for any other distros?
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 don't think so, at least not for any of the distros we currently support (redhat, alpine, and derivatives of those), SourceName is not in either of those metadatas.
internal/imodels/imodels.go
Outdated
return pkg.Inventory.Version | ||
} | ||
|
||
func (pkg *PackageInfo) SetVersion() string { |
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.
This looks like it's unnecessary? It's not realy setting any values and hte contents are the same as Verison()
.
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.
Ah must have been a copy and paste error, removed it now.
e5b9211
to
20b4ff3
Compare
This PR refactors PackageInfo to be accessed via methods, which will perform necessary transformations on the underlying inventory as needed.
This also adds the ecosystemmock extractor which allows you set the return value of the Ecosystem() function.
TODO: Add tests for imodels. I will add tests in a followup PR once we are confident we are sticking with this approach and it works with the other refactors.