-
-
Notifications
You must be signed in to change notification settings - Fork 533
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
Fix licenses in HTML output #3275
Conversation
Fix an issue in HTML output where no licenses were being shown, now we also use license references here. Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
9b77403
to
49e7d89
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, but we need a test
Hi, license text is not included in HTML-output. As I am new to scancode I am not sure if this is expected behavior. |
@jbohrmann can you confirm that you used the checkout of this branch?
I seem to have licenses fine from this branch, attaching here the output.html file I generated using these commands above: Also
|
Yes I can confirm.
I preferred HTML as I can pass the output easily to get legal advice. |
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@jbohrmann what is your OS? also what is your installation process? Is it a git checkout or an app archive download? the .VERSION file looks like this https://github.com/nexB/scancode-toolkit/blob/develop/.VERSION when you are installing from a git checkout, what do you mean info from this file? See above the commit with added test for licenses in html, looks like they work fine now after these changes ^ |
@AyanSinhaMahapatra @jbohrmann said it was Windows and Python 3.9 ;)
|
but afaics, we do not include the license text be it the matched text or the reference text in the standard HTML output. Maybe we should. HTML is nicer than JSON for sure. |
@pombredanne We could have support for license-text, summary, license-clarity-score and others too in the HTML output, I opened an issue for this: #3281. |
I downloaded the branch as zip over Github. OS is Windows 10. |
@jbohrmann you are right! Note that there are a few alternatives:
|
* Add named arguments and license_references as applicable * Make --html-app a hidden argument * Add homepage_url and tests for the same Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
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. I've tested out the HTML output on a scan of the samples directory and the results look great. Thanks!
Thanks @JonoYang , merging! |
Fixes #3272
Tasks
Run tests locally to check for errors.