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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Follow license reference to another file #2616

Merged
merged 7 commits into from
Aug 27, 2021
Merged

Follow license reference to another file #2616

merged 7 commits into from
Aug 27, 2021

Conversation

akugarg
Copy link
Collaborator

@akugarg akugarg commented Jul 30, 2021 •
edited
Loading

Signed-off-by: akugarg akanksha.garg2k@gmail.com

Fixes #1364

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 📁

@akugarg
Create plugin for following reference files
5fa4d00 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
@akugarg
Add new test cases
5e4928d 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
@akugarg
Copy link
Collaborator Author

akugarg commented Aug 3, 2021

Hey! @AyanSinhaMahapatra How we will go about adding same logic for packagecode like we discussed yesterday?

AyanSinhaMahapatra
AyanSinhaMahapatra requested changes Aug 4, 2021
View reviewed changes
src/licensedcode/plugin_license.py Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
tests/licensedcode/test_plugin_license.py Show resolved Hide resolved
tests/licensedcode/data/plugin_license/license_reference/scan/license-notice.txt Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Show resolved Hide resolved
src/licensedcode/plugin_license.py Show resolved Hide resolved
@AyanSinhaMahapatra
Copy link
Member

Also, for packagedcode, say for npm, see here. We have a function converting a list of license declarations to a license-expression. It would have been ideal if we return some form of License Detections here instead of just the expressions (which is WIP), but in a short term we could check first for these kind of license references to file (and report only the license conclusion without the unknown).

@akugarg
Update test cases upto expectations
4aab0aa 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
This was referenced Aug 9, 2021
Closed
Merged
@akugarg akugarg force-pushed the follow_reference branch 2 times, most recently from 3200da4 to d4f1436 Compare August 14, 2021 09:09
@akugarg
Copy link
Collaborator Author

akugarg commented Aug 14, 2021

@AyanSinhaMahapatra @pombredanne Please have a look!
Also Can we keep the reference part for npm pacakges in a different PR from this one ?

pombredanne
pombredanne requested changes Aug 15, 2021
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! beside the review comments, I would also like to see some unit tests for find_reference_licenses

src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
@akugarg akugarg force-pushed the follow_reference branch 3 times, most recently from 10385da to 27146ae Compare August 16, 2021 11:02
@akugarg
define process_codebase function for matching references
a84b1ed 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
@pombredanne pombredanne mentioned this pull request Aug 25, 2021
Closed
pombredanne
pombredanne requested changes Aug 25, 2021
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few final review items for you. Thanks!

src/licensedcode/plugin_license.py Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
src/licensedcode/plugin_license.py Outdated Show resolved Hide resolved
@pombredanne pombredanne mentioned this pull request Aug 26, 2021
Open
@akugarg
Add unit tests
466e375 
Signed-off-by: akugarg <akanksha.garg2k@gmail.com>
@pombredanne
Fix minor typo in comment
6aa5d9d 
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
pombredanne
pombredanne approved these changes Aug 27, 2021
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM... Thank you ++

@pombredanne pombredanne merged commit cee6d68 into nexB:develop Aug 27, 2021
@pombredanne pombredanne mentioned this pull request Aug 27, 2021
Merged
4 tasks
@akugarg akugarg deleted the follow_reference branch August 27, 2021 14:16
pombredanne added a commit that referenced this pull request Aug 29, 2021
@pombredanne
Do not match referenced_filenames path suffixes #1364
cdd4c0e 
Only follow license references match an exact filename
In #2616 we introduced matching path of referenced_filenames
based on matching filename or path suffix. This removes path suffix
matching which is problematic.

Before this we were using .endswith(path) and this led to weird and
incorrect license dereferences

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne pombredanne mentioned this pull request Aug 29, 2021
Merged
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Feb 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AyanSinhaMahapatra AyanSinhaMahapatra AyanSinhaMahapatra requested changes

@pombredanne pombredanne pombredanne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
GSoC 2021: Improve Unknown License De...
Done
Milestone
No milestone
3 participants
@akugarg @AyanSinhaMahapatra @pombredanne