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

deps,tools: add missing nghttp2 license #14806

Closed
wants to merge 1 commit into from
Closed

deps,tools: add missing nghttp2 license #14806

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Add COPYING from nghttp2 and register it in tools/license-builder.sh.

Also run tools/license-builder.sh and commit the resulting LICENSE file.

Ref: https://github.com/nghttp2/nghttp2/blob/master/COPYING

Checklist

@addaleax
deps,tools: add missing nghttp2 license
6f84616 
Add `COPYING` from `nghttp2` and register it in
`tools/license-builder.sh`.

Also run `tools/license-builder.sh` and commit the
resulting `LICENSE` file.

Ref: https://github.com/nghttp2/nghttp2/blob/master/COPYING
@addaleax addaleax added http2 Issues or PRs related to the http2 subsystem. meta Issues and PRs related to the general management of the project. labels Aug 13, 2017
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Aug 13, 2017
refack
refack approved these changes Aug 13, 2017
View reviewed changes
Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

LGTM, but probably good if a lawer takes a look.

@addaleax
Copy link
Member Author

@refack I think if there is a problem with this PR, then there is a problem with how we handle dependency licenses in general.

If you think it is necessary or helpful for a lawyer to take a look, bring that up with on the TSC repository, because that’s the only way for us to contact the Foundation’s legal committee. In that case, it would also be good to know about any more specific concerns, if you have any.

@refack
Copy link
Contributor

refack commented Aug 13, 2017

The only "possible issue" I see is the timing of this PR, it probably should have been part of #14239.
Apart from that, and from my own limited personal experience it looks good as is.

@addaleax
Copy link
Member Author

Fwiw this is also how we handled it for the v8_inspector dependency, as far as I recall.

The only "possible issue" I see is the timing of this PR, it probably should have been part of #14239.

Should be fine as long as our releases do contain the license. Also, what’s the alternative at this point? ;)

@jasnell
Copy link
Member

jasnell commented Aug 13, 2017

Oy, good catch :-)

@addaleax
Copy link
Member Author

Landed in afe68c1 because we can’t release HTTP/2 without it. ;) Thanks for all of the quick reviews.

@addaleax addaleax closed this Aug 14, 2017
@addaleax addaleax deleted the nghttp2-license branch August 14, 2017 15:23
addaleax added a commit that referenced this pull request Aug 14, 2017
@addaleax
deps,tools: add missing nghttp2 license
afe68c1 
Add `COPYING` from `nghttp2` and register it in
`tools/license-builder.sh`.

Also run `tools/license-builder.sh` and commit the
resulting `LICENSE` file.

Ref: https://github.com/nghttp2/nghttp2/blob/master/COPYING
PR-URL: #14806
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
addaleax added a commit that referenced this pull request Aug 14, 2017
@addaleax
deps,tools: add missing nghttp2 license
dd521d0 
Add `COPYING` from `nghttp2` and register it in
`tools/license-builder.sh`.

Also run `tools/license-builder.sh` and commit the
resulting `LICENSE` file.

Ref: https://github.com/nghttp2/nghttp2/blob/master/COPYING
PR-URL: #14806
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@addaleax addaleax mentioned this pull request Aug 14, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tniessen tniessen tniessen approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

@danbev danbev danbev approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
http2 Issues or PRs related to the http2 subsystem. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@addaleax @refack @jasnell @danbev @lpinca @TimothyGu @tniessen @MylesBorins @nodejs-github-bot