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

License builder tool #4194

Merged
merged 2 commits into from
Jan 14, 2016
Merged

License builder tool #4194

merged 2 commits into from
Jan 14, 2016

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Dec 8, 2015

Added tools/license-builder.sh to construct the LICENSE file from the licenses of all the dependencies used by Node.

Re-ordered the licenses alphabetically within dependency type groups:

  • Dependencies bundled in Node.js
    • C-Ares
    • HTTP Parser
    • ICU
    • libuv
    • OpenSSL
    • Punycode.js
    • V8
    • libuv (newly included)
  • npm
  • Build tools
    • GYP
    • marked
  • Test tools
    • cpplint.py
    • ESLint
    • gtest
    • node-weak

Additionally:

  • Cleaned up naming to be consistent with what each of these projects appear to want to be called.
  • Made the lead for each license consistent NAME, located at LOCATION, is licensed as follows:
  • Stripped out comment syntax, HTML and other unnecessary guff from licenses that have them (e.g. where they are included in source files and need it to be in comments)

It's surprising how far out of sync we are with our dependency licenses cause we don't have a tool or procedure to keep them updated.

This probably needs to wait at least until we get results from #3979 but I'd like us to have a tool that keeps our rules consistent and lets us ensure our LICENSE is up to date.

/cc @mikeal

@rvagg
Copy link
Member Author

rvagg commented Dec 8, 2015

renamed "C-Ares" to "c-ares", realised that I can't find reference to that capitalisation on the internet

@mscdex mscdex added the meta Issues and PRs related to the general management of the project. label Dec 8, 2015
@jasnell
Copy link
Member

jasnell commented Dec 11, 2015

Rubber stamp LGTM

@rvagg
Copy link
Member Author

rvagg commented Jan 13, 2016

@nodejs/ctc anyone want to weigh in here? the work here is for consistency (lots of variation in how licenses are introduced and presented) and completeness (some are missing) with a bit of reordering according to the grouping above. I haven't touched the top bit, leaving that for when we finally get legal advice on it but tbh I'm not holding my breath for that happening. Once we have a tool in place, we can adjust it according to any official advice we get from lawyers.

@jasnell
Copy link
Member

jasnell commented Jan 13, 2016

Nothing further really. Let's get the PR landed and get the tool in place, we can tweak and iterate from there.

@Fishrock123
Copy link
Contributor

rubber-stamp lgtm

rvagg added 2 commits January 14, 2016 22:07
PR-URL: nodejs#4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
PR-URL: nodejs#4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg added a commit that referenced this pull request Jan 14, 2016
PR-URL: #4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg added a commit that referenced this pull request Jan 14, 2016
PR-URL: #4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@rvagg
Copy link
Member Author

rvagg commented Jan 14, 2016

Landed @ 031b87d & d91646b, improvements can come via PR.

OSX installer is as ugly as ever, no surprises there

screen shot 2016-01-14 at 11 19 45 pm

The Windows installer is meant to use an RTF rendering of this doc, hence the additional formatting in there. I don't have a Windows machine handy right now due to a big office cleanup, could someone from @nodejs/platform-windows please grab an msi from https://nodejs.org/download/nightly/v5.4.2-nightly20160114e855b596f4/ and screenshot the license screen for us?

@rvagg
Copy link
Member Author

rvagg commented Jan 14, 2016

fwiw I'm hoping we can deal with the OSX formatting along with #2571 but I haven't looked at it yet (which is why it's still in limbo!)

@seishun
Copy link
Contributor

seishun commented Jan 14, 2016

@rvagg This one?
nodelicense

@rvagg
Copy link
Member Author

rvagg commented Jan 14, 2016

Yep, that looks OK I think @seishun, if you do a scroll through does it look ok (as far as these things go), i.e. proper formatting rather than markdown?

@seishun
Copy link
Contributor

seishun commented Jan 14, 2016

Does this count as proper formatting?
license2
Otherwise everything looks ok.

@rvagg
Copy link
Member Author

rvagg commented Jan 14, 2016

bah, that doesn't look right .. probably need to tweak either the LICENSE.md format or the rtf converter.

Will leave this issue open as a reminder, to myself or anyone else wanting to tackle this job (hint: ./tools/license2rtf.js).

@rvagg
Copy link
Member Author

rvagg commented Jan 14, 2016

and thanks for being responsive on this @seishun!

@rvagg rvagg merged commit d91646b into nodejs:master Jan 14, 2016
rvagg added a commit that referenced this pull request Jan 28, 2016
PR-URL: #4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg added a commit that referenced this pull request Jan 28, 2016
PR-URL: #4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
PR-URL: #4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
PR-URL: #4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
PR-URL: nodejs#4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
PR-URL: nodejs#4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
PR-URL: nodejs#4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
PR-URL: nodejs#4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
PR-URL: nodejs#4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
PR-URL: nodejs#4194
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants