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

tools: remove obsolete entries from license #21979

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jul 25, 2018

The LICENSE file has a few entries for things that no longer ship with
the code base. They are installed via npm instead. Remove them from the
license file.

Running the license builder on a fresh checkout will
result in errors until this change lands, since the necessary
information is not in the source tree until the npm install happens.

Checklist

The LICENSE file has a few entries for things that no longer ship with
the code base. They are installed via npm instead. Remove them from the
license file.

Running the license builder on a fresh checkout will
result in errors until this change lands, since the necessary
information is not in the source tree until the `npm install` happens.
@Trott Trott added the tools Issues and PRs related to the tools directory. label Jul 25, 2018
@nodejs-github-bot nodejs-github-bot added meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Jul 25, 2018
@Trott
Copy link
Member Author

Trott commented Jul 25, 2018

@nodejs/build-files

@Trott
Copy link
Member Author

Trott commented Jul 25, 2018

I think the lite CI run is sufficient here since none of this code gets exercised in CI, but if someone thinks a full run is required, go for it.

@@ -85,8 +83,6 @@ addlicense "gtest" "deps/gtest" "$(cat ${rootdir}/deps/gtest/LICENSE)"
addlicense "nghttp2" "deps/nghttp2" "$(cat ${rootdir}/deps/nghttp2/COPYING)"

# remark
addlicense "unified" "deps/unified" "$(cat ${rootdir}/tools/doc/node_modules/unified/LICENSE)"
addlicense "remark-rehype" "deps/remark-rehype" "$(cat ${rootdir}/tools/doc/node_modules/remark-rehype/LICENSE)"
Copy link
Member

Choose a reason for hiding this comment

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

Just noting here that these were presumably wrong anyway (i.e. the location should have been tools/doc/node_modules/*.

@Trott
Copy link
Member Author

Trott commented Jul 26, 2018

👍 this comment to fast-track.

@Trott Trott added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 26, 2018
Copy link
Contributor

@sagirk sagirk left a comment

Choose a reason for hiding this comment

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

LGTM!

Trott added a commit to Trott/io.js that referenced this pull request Jul 26, 2018
The LICENSE file has a few entries for things that no longer ship with
the code base. They are installed via npm instead. Remove them from the
license file.

Running the license builder on a fresh checkout will
result in errors until this change lands, since the necessary
information is not in the source tree until the `npm install` happens.

PR-URL: nodejs#21979
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@Trott
Copy link
Member Author

Trott commented Jul 26, 2018

Landed in 40a413e

@Trott Trott closed this Jul 26, 2018
targos pushed a commit that referenced this pull request Jul 31, 2018
The LICENSE file has a few entries for things that no longer ship with
the code base. They are installed via npm instead. Remove them from the
license file.

Running the license builder on a fresh checkout will
result in errors until this change lands, since the necessary
information is not in the source tree until the `npm install` happens.

PR-URL: #21979
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@targos targos mentioned this pull request Jul 31, 2018
@Trott Trott deleted the update-license branch January 13, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants