Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add copyright holder field to PackageData model #3302

Merged
merged 6 commits into from
Apr 6, 2023
Merged

Add copyright holder field to PackageData model #3302

merged 6 commits into from
Apr 6, 2023

Conversation

keshav-space
Copy link
Member

@keshav-space keshav-space commented Mar 27, 2023 •
edited

Fixes #3290

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 馃搼 and links the original issue above 馃敆
  • Tests pass -- look for a green checkbox 鉁旓笍 a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 馃搧

AyanSinhaMahapatra
AyanSinhaMahapatra requested changes Mar 29, 2023
View reviewed changes
Copy link
Member

@AyanSinhaMahapatra AyanSinhaMahapatra left a 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?

src/packagedcode/models.py Outdated Show resolved Hide resolved
src/packagedcode/models.py Outdated Show resolved Hide resolved
@keshav-space
Add copyright holder field to PackageData model nexB#3290
b59f80a 
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
@keshav-space
Fix failing tests
0baa787 
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
@keshav-space
Re-run heisenbug in DEBUG mode
b665ade 
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
@keshav-space
Refine holder detection
795ec30 
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
@keshav-space keshav-space force-pushed the 3290_package_holder branch 2 times, most recently from 01e3467 to 563baf8 Compare March 31, 2023 06:55
AyanSinhaMahapatra
AyanSinhaMahapatra requested changes Mar 31, 2023
View reviewed changes
Copy link
Member

@AyanSinhaMahapatra AyanSinhaMahapatra left a 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
@keshav-space keshav-space force-pushed the 3290_package_holder branch 5 times, most recently from 847055b to abdcd35 Compare March 31, 2023 12:35
@keshav-space
Regenerate test fixtures
5151edc 
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
AyanSinhaMahapatra
AyanSinhaMahapatra approved these changes Apr 3, 2023
View reviewed changes
Copy link
Member

@AyanSinhaMahapatra AyanSinhaMahapatra left a 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.

tests/summarycode/data/tallies/full_tallies/tallies_key_files-details.expected.json-lines
@@ -20,11 +20,11 @@
"warnings": [],
"extra_data": {
"system_environment": {
"operating_system": "linux",
Copy link
Member

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.

@keshav-space
Copy link
Member Author

We need a changelog entry for this and we're ready to merge.

@AyanSinhaMahapatra Changelog for this will go under v32.0.0 ?

@AyanSinhaMahapatra
Copy link
Member

@keshav-space yes! No need to bump any version since we're still doing release candidates.

@keshav-space
Update CHANGELOG nexB#3290
655874a 
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
JonoYang
JonoYang approved these changes Apr 5, 2023
View reviewed changes
Copy link
Contributor

@JonoYang JonoYang left a 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!

@AyanSinhaMahapatra
Copy link
Member

@keshav-space Thanks++
Merging!

@AyanSinhaMahapatra AyanSinhaMahapatra merged commit 013e846 into nexB:develop Apr 6, 2023
30 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonoYang JonoYang JonoYang approved these changes

@AyanSinhaMahapatra AyanSinhaMahapatra AyanSinhaMahapatra approved these changes

@pombredanne pombredanne Awaiting requested review from pombredanne

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Copyright Holder information for Packages
3 participants
@keshav-space @AyanSinhaMahapatra @JonoYang