-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
doc: add supported platforms list #11872
Conversation
BUILDING.md
Outdated
|
||
### Shared libraries & dependencies | ||
|
||
Node.js intends to support building against shared representations of dependencies found in the [*deps*][./deps/] directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[*deps*](./deps/)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
BUILDING.md
Outdated
| GNU/Linux | Tier 1 | kernel >= 2.6.18, glibc >= 2.5 | x86, x64, arm, arm64 | | | ||
| macOS | Tier 1 | >= 10.10 | x64 | | | ||
| Windows | Tier 1 | >= Windows 7 or >= Windows2008R2 | x86, x64 | | | ||
| SmartOS | Tier 2 | >= 14 < 16.4 | x86, x64 | see note1 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On master, I think it's >=15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: can you wrap lines at 80 columns?
BUILDING.md
Outdated
|
||
#### Unix | ||
|
||
* GCC 4.8 or newer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd specify this as 4.8.5 per the discussion in #11840 and nodejs/v8#5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I guess we can then fix up for earlier versions in the backports
BUILDING.md
Outdated
|
||
#### Windows | ||
|
||
* Building Node: Visual Studio 2015¹ or Visual C++ Build Tools 2015 or newer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this <sup>1</sup>
trying to link to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nowhere that I can find, will remove.
@targos @bnoordhuis @vsemozhetbyt @Fishrock123 addressed initial set of commments. @bnoordhuis I wrapped at 80 were I thought that would work. I'm guessing it won't for the table although I did not have time today to try that. Do you believe that should be able to be wrapped as well and not mess up the table formatting ? |
Needs a quick rebase |
BUILDING.md
Outdated
### Shared libraries & dependencies | ||
|
||
Node.js intends to support building against shared representations of | ||
dependencies found in the [*deps*](./deps/) directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd leave this out. With c-ares it was broken for years. The numerous V8 patches we float basically mean it's only guaranteed to work with our fork.
Historically, we made no promises except that we'll consider patches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove for now, and we can add back in through follow PRs if appropriate after more discussion. What I think makes sense is that once this PR lands, I can open a new one adding that back in and we can have the discussion there as opposed to blocking getting the reset of the info in.
@mhdawson Leaving the table unchanged is fine. |
@mhdawson I see that the original PR was targeting v6, should the target branch of this one be changed to v6, or is it the supported platforms list for Node 8 (or Node 9, as there's already a v8.x branch)? The other question is how this will be updated, is the plan to make sure a commit lands revising it before each major release of Node? |
@gibfahn my plan is to land this for master as a base and then submit PRs to the other branches which reflect what is supported for those levels. In terms of new releases, I think we should try to keep it up to date generally but definitely make sure it is updated to just before each major release of Node. |
Ok rebased, and addressed @bnoordhuis comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, this has been open since October, let's land it.
BUILDING.md
Outdated
#### Windows | ||
|
||
* Building Node: Visual Studio 2015 or Visual C++ Build Tools 2015 or newer | ||
* Building native add-ons: Visual Studio 2013 or Visual C++ Build Tools 2015 or newer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long line.
Ok fixed long line, will now land. |
CI run, to be safe |
PR-URL: #11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
Landed as ef47687. Next steps
|
- In the review of nodejs#11872 we pulled out the discussion of supporting dependencies. This PR adds back that statement. - Update README.md to indicatte BUILDING.md contains the list of supported platforms.
PR for discussion around what we should say about supporting dependencies: #11942 |
Closing this PR as the commit has landed in ef47687 |
PR to add supported platform list to 6.X - #11943 |
PR-URL: nodejs#11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
PR-URL: nodejs#11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
Original Commit Message: PR-URL: #11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Backport-Of: #11872 PR-URL: #11943 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original Commit Message: PR-URL: #11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Backport-Of: #11872 PR-URL: #11943 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original Commit Message: PR-URL: nodejs#11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Backport-Of: nodejs#11872 PR-URL: nodejs#11943 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original Commit Message: PR-URL: #11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Backport-Of: #11872 PR-URL: #11943 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original Commit Message: PR-URL: #11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Backport-Of: #11872 PR-URL: #11943 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original Commit Message: PR-URL: #11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Backport-Of: #11872 PR-URL: #11943 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original Commit Message: PR-URL: nodejs/node#11872 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Backport-Of: nodejs/node#11872 PR-URL: nodejs/node#11943 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
doc
Talked to Rod, going to take over landing the supported platform list from him. Earlier review/discussion was in: d883278