-
-
Notifications
You must be signed in to change notification settings - Fork 594
Add copyright holder field to PackageData model #3302
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
Add copyright holder field to PackageData model #3302
Conversation
9e97760
to
ca5c4c8
Compare
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 @keshav-space see comments for changes.
Also not that we only have copyrights detected for debian packages only, I could not see any issues for detecting copyrights for all other supported package manifests, apart from this #2912, we should look into this and have issues to add copyright/holder support for all packages.
@pombredanne @JonoYang what fo you think?
Signed-off-by: Keshav Priyadarshi <[email protected]>
Signed-off-by: Keshav Priyadarshi <[email protected]>
Signed-off-by: Keshav Priyadarshi <[email protected]>
ca5c4c8
to
9ae5653
Compare
Signed-off-by: Keshav Priyadarshi <[email protected]>
01e3467
to
563baf8
Compare
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.
See comments for your consideration, thanks!
tests/packagedcode/data/alpine/apkbuild-problems/alpine14/main/sudo/APKBUILD-expected.json
Outdated
Show resolved
Hide resolved
847055b
to
abdcd35
Compare
Signed-off-by: Keshav Priyadarshi <[email protected]>
abdcd35
to
5151edc
Compare
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.
LGTM! Thanks ++ @keshav-space
We need a changelog entry for this and we're ready to merge.
@@ -20,11 +20,11 @@ | |||
"warnings": [], | |||
"extra_data": { | |||
"system_environment": { | |||
"operating_system": "linux", |
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.
Minor nitpick for next time, we can avoid this churn on force regen by either removing the header if not necessary for test purposes, or just not commit these changes if possible. Don't have to change anything for now.
@AyanSinhaMahapatra Changelog for this will go under v32.0.0 ? |
@keshav-space yes! No need to bump any version since we're still doing release candidates. |
Signed-off-by: Keshav Priyadarshi <[email protected]>
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.
@keshav-space Looks good, thanks!
@keshav-space Thanks++ |
Fixes #3290
Tasks
Run tests locally to check for errors.