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.

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

Auto-generate license file to contain licenses of bundled dependencies #3063

Merged
merged 1 commit into from Aug 21, 2019

Conversation

lukastaegert
Copy link
Member

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

When bundling dependencies, it is possible that consumers of your software install and use code that has a different license than what they thought they are installing. Furthermore, most comonly used licenses (e.g. MIT, ISC, Apache) require you to add a copy of the license text when distributing the code.

Until now, Rollup has not been upfront about the licenses of the bundled dependencies and was also not in compliance with the requirement to add the proper license texts.

To solve this, I have been in communications with @mjeanroy, who has been so awesome to extend rollup-plugin-license to

  • use tree-shaking information to collect all licenses of dependencies you actually use in your bundle
  • provide a callback hook that can be used to assemble a license file containing the license texts + additional information about all bundled dependencies.

I have changed the rollup build so that the license file is updated each time we do a full build. If there are changes, the changed license file should be committed again.

Feedback is very much welcome. My hope is that this could serve as an example on how to handle licenses when bundling dependencies.

@codecov
Copy link

codecov bot commented Aug 16, 2019

Codecov Report

Merging #3063 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3063   +/-   ##
=======================================
  Coverage   88.75%   88.75%           
=======================================
  Files         165      165           
  Lines        5737     5737           
  Branches     1748     1748           
=======================================
  Hits         5092     5092           
  Misses        388      388           
  Partials      257      257

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9879cbc...f3adad8. Read the comment docs.

@lukastaegert lukastaegert changed the title Auto-generate license file containing licenses of bundled dependencies Auto-generate license file to contain licenses of bundled dependencies Aug 16, 2019
@lukastaegert
Copy link
Member Author

Ok, seems we need to find a good way to not parse the plugin on Node-6. Will have a look after the weekend.

@mjeanroy
Copy link
Contributor

@lukastaegert Sorry, I did not see that I broke compatibility with node 6. I think that, as along as rollup support node 6, plugins should too, so I'm going to fix this.

@lukastaegert
Copy link
Member Author

Oh cool! I think I could work around it by using a conditional dynamic import until we probably drop Node 6 some time later this year with Rollup 2 but that would be easier 😉

@shellscape
Copy link
Contributor

shellscape commented Aug 16, 2019

You got bit by a spread operator on Node 6. We ought to merge this, and then drop Node 6 given that it's out of maintenance LTS now. That'll be a major version, but that's NBD.

Whoops, the site wasn't showing previous replies.

@lukastaegert we should make dropping node 6 a priority imho. It's been out of maintenance for some time now. And a new major version isn't a big deal, lots of numbers left :)

@lukastaegert
Copy link
Member Author

True, but I would not want to spin the major too easily either. A major is both an opportunity for publicity as well as something where people will only update cautiously. We should at least make sure we collect all pending depreciations as well as make up our minds which features that would require a major version we want to add. Then all depreciations need to be properly documented with their recommended upgrade paths etc. Always a lot of work involved.

@lukastaegert
Copy link
Member Author

Also, all plugins in the rollup organization should at least receive a small audit once we have a 2.0 branch.

@shellscape
Copy link
Contributor

(I'm way off-topic so apologies to those tracking the thread) Fwiw the plugins are on my radar next. Get ready for more notification emails haha.

@lukastaegert
Copy link
Member Author

Fixed the Node 6 compatibility issue for now.

@mjeanroy
Copy link
Contributor

@lukastaegert Hi, rollup-plugin-license is now compatible with node 6, use version 0.12.1 and it should be ok.

@lukastaegert
Copy link
Member Author

Nice, updated to 12.1, working well!

@lukastaegert
Copy link
Member Author

Merging as I think there were no concerns with the PR in its essence any more.

@lukastaegert lukastaegert merged commit 852e704 into master Aug 21, 2019
@lukastaegert lukastaegert deleted the generate-license-file branch August 21, 2019 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants