Update Node.js readme to avoid confusion #13
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 6.2. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
regress/regress-crbug-514081 allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test Fixes: nodejs/node#6340 PR-URL: nodejs/node#6678 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Fedor Indutny <fedor@indutny.com>
VS2013 does not support defaulting move constructor and assignment operator. This adds explicit definitions of those methods for two classes. This fix is required because we still support building addons with VS2013 and the incompatibility is in v8.h. Fixes: nodejs#4 PR-URL: nodejs/node#13263 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
|
I think it's OK to delete the README but it has to be done in the update script, somehow. This branch is overwritten by it. |
|
What we can do is discuss here about the contents and I'll add a step to the script that replaces the readme with it. |
|
Sounds good! |
TimothyGu
approved these changes
Jul 25, 2017
README.md
Outdated
| <a title="CII Best Practices" href="https://bestpractices.coreinfrastructure.org/projects/29"><img src="https://bestpractices.coreinfrastructure.org/projects/29/badge"></a> | ||
| </p> | ||
| This is a mirror of [Node.js](https://github.com/nodejs/node) | ||
| with V8 Tip-of-Tree. |
There was a problem hiding this comment.
I'd explicitly call this out as experimental:
This is an automatically updated experimental version of Node.js with V8 Tip-of-Tree embedded.
LGTM either way though.
There was a problem hiding this comment.
Thanks, that's a great suggestions.
04083a9 to
4769085
Compare
We want to make sure nobody confuses this repo with the main repo. Ref: nodejs#6
504d70a to
0edb9cf
Compare
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want to make sure nobody confuses this repo with the main repo.
Ref: #6
Any feelings on whether it's OK to delete the original Readme? Should we keep it and just prefix it with the explanation?