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

Copy license to build output directory. #2966

Merged
merged 1 commit into from
Apr 9, 2018

Conversation

adunkman
Copy link
Contributor

@adunkman adunkman commented Apr 3, 2018

@harvesthq/chosen-developers to fix #2945.

This adds a step to the build grunt task which copies LICENSE.md to the output directory… which should theoretically include it in the final build? I’m not 100% sure, because build process is a little 🌁.

Also, adds a package-lock.json, since it seems like we haven’t touched the dependencies in a while, and that’s a thing now.

bitmoji

Copy link
Collaborator

@koenpunt koenpunt left a comment

Choose a reason for hiding this comment

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

Do we also need to add a license property to the package.json?

Gruntfile.coffee Outdated
copy:
main:
files: [
{src: ['LICENSE.md'], dest: 'public/'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curly braces are not necessary here, and since we don't use them anywhere in the Gruntfile I suggest we remove them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or even better, use the simplified, single-file syntax;

copy:
  main:
    src: 'LICENSE.md'
    dest: 'public/'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, I didn’t catch that syntax in the docs. I’ll update momentarily!

@stof
Copy link
Collaborator

stof commented Apr 4, 2018

Should we also update the package.json of the built package, to include the license file in the npm archive ?

@adunkman
Copy link
Contributor Author

adunkman commented Apr 4, 2018

LICENSE (with any capitalization or extension) is automatically included in packages by npm — sorry, I should have mentioned that in the description (I had to look it up!):

Certain files are always included, regardless of settings:

  • LICENSE / LICENCE

README, CHANGES, LICENSE & NOTICE can have any case and extension.

@stof
Copy link
Collaborator

stof commented Apr 4, 2018

then it is fine

Also, adds a package-lock.json, since it seems like we haven’t touched the dependencies in a while.
@adunkman adunkman force-pushed the add-license-to-build-output branch from 16ac889 to b0ac854 Compare April 9, 2018 15:49
@adunkman
Copy link
Contributor Author

adunkman commented Apr 9, 2018

Force-pushed an updated commit with the Gruntfile modifications above. Thanks for the review!

@adunkman adunkman merged commit b0ac854 into master Apr 9, 2018
@adunkman adunkman deleted the add-license-to-build-output branch April 9, 2018 16:02
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.

License text missing from chosen-package
3 participants