-
Notifications
You must be signed in to change notification settings - Fork 47
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
Added '--from=all' option #86
Conversation
* list both metadata and classifier license information
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
+ Coverage 98.86% 98.87% +0.01%
==========================================
Files 1 1
Lines 353 357 +4
==========================================
+ Hits 349 353 +4
Misses 4 4
Continue to review full report at Codecov.
|
@cdce8p Thanks for the info on PEP 639. I will review and feedback on your branch at a later date.
Yes, this will be a task I will tackle when I release the next version 😉 |
This discussion is also related to issue #72 . |
license_from_classifier = ', '.join(licenses) | ||
|
||
return license_from_classifier | ||
return licenses |
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.
Just my little opinion, the return value of the function find_license_from_classifier
was changed from str
to list
, which made other implementation and test codes more complicated.
But it's not important enough to oppose the merge.
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.
You're right that it made the commit a bit more complicated. However I believe this is the better solution here. Changing the responsibility of this function to only extract the license classifier might make the work down the line a bit easier.
@cdce8p I've checked your patch. I'm glad you worked with attention to compatibility. I am positive that I will merge this patch. I hope to move on to the next step. |
Thanks for taking the time to review my changes. I went ahead and added a test case for the change. |
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.
Thank you for your contribution.
This feature will be in the next release, 3.2.0.
Version 3.2.0 has just been released! |
At the moment
pip-licenses
can only output either the metadata or the classifier license information.This PR adds the
--from=all
option that displays both license information at the same time .Background
PEP 639 specified how Python packages are supposed to define license information going forward. The preferred way is to use the
license
metadata field with SPDX license expression syntax. Although not officially deprecated, the trove classifiers will slowly be phased out and won't receive any more updates.Since the PEP is only from Aug-2019, most packages still use the trove class and/or have outdated/invalid metadata license information. With the proposed option, it will be easier to compare the used licenses.
Going forward
--from=mixed
still prefers the classifier over metadata information. Changing this would be a big breaking change and probably confuse many users. Therefore I'm not sure if it will be worth it. At least not for now.license
information forpip-licenses
to justMIT
to conform with the SPDX identifier.Ideas
fail
/allow
--
Next steps for this PR