FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: add supported platforms list by mhdawson · Pull Request #11872 · nodejs/node · GitHub

/ node Public

doc: add supported platforms list - #11872

Closed
mhdawson wants to merge 2 commits into
nodejs:masterfrom
mhdawson:docsupport
ConversationCommits (2)ChecksFiles changed
Closed

doc: add supported platforms list#11872
mhdawson wants to merge 2 commits into
nodejs:masterfrom
mhdawson:docsupport

Conversation

mhdawson commented Mar 15, 2017

Copy link
Copy Markdown
Member

Checklist

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Talked to Rod, going to take over landing the supported platform list from him. Earlier review/discussion was in: d883278

mhdawson self-assigned this Mar 15, 2017
nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 15, 2017
vsemozhetbyt reviewed Mar 15, 2017
View reviewed changes
Comment thread BUILDING.md Outdated

vsemozhetbyt Mar 15, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

[*deps*](./deps/) ?

mhdawson Mar 16, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

done.

targos reviewed Mar 16, 2017
View reviewed changes
Comment thread BUILDING.md Outdated

targos Mar 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

On master, I think it's >=15

mhdawson Mar 16, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

will update

bnoordhuis requested changes Mar 16, 2017
View reviewed changes

bnoordhuis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Aside: can you wrap lines at 80 columns?

Comment thread BUILDING.md Outdated

bnoordhuis Mar 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd specify this as 4.8.5 per the discussion in #11840 and nodejs/v8#5.

mhdawson Mar 16, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

ok, I guess we can then fix up for earlier versions in the backports

Fishrock123 reviewed Mar 16, 2017
View reviewed changes
Comment thread BUILDING.md Outdated

Fishrock123 Mar 16, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

where is this <sup>1</sup> trying to link to?

mhdawson Mar 16, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

nowhere that I can find, will remove.

mhdawson commented Mar 16, 2017

Copy link
Copy Markdown
Member Author

@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 ?

jasnell commented Mar 16, 2017

Copy link
Copy Markdown
Member

Needs a quick rebase

bnoordhuis requested changes Mar 17, 2017
View reviewed changes
Comment thread BUILDING.md Outdated

bnoordhuis Mar 17, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

mhdawson Mar 17, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

bnoordhuis commented Mar 17, 2017

Copy link
Copy Markdown
Member

@mhdawson Leaving the table unchanged is fine.

gibfahn commented Mar 17, 2017

Copy link
Copy Markdown
Member

@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?

mhdawson commented Mar 17, 2017 •
edited
Loading

Copy link
Copy Markdown
Member Author

@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.

mhdawson commented Mar 17, 2017

Copy link
Copy Markdown
Member Author

Ok rebased, and addressed @bnoordhuis comment.

gibfahn reviewed Mar 17, 2017
View reviewed changes

gibfahn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, this has been open since October, let's land it.

bnoordhuis approved these changes Mar 17, 2017
View reviewed changes
Comment thread 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

bnoordhuis Mar 17, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Long line.

mhdawson commented Mar 20, 2017

Copy link
Copy Markdown
Member Author

Ok fixed long line, will now land.

mhdawson commented Mar 20, 2017

Copy link
Copy Markdown
Member Author

CI run, to be safe

CI: https://ci.nodejs.org/job/node-test-pull-request/6935/

mhdawson added a commit that referenced this pull request Mar 20, 2017
doc: add supported platforms list
ef47687 
PR-URL: #11872
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

mhdawson commented Mar 20, 2017

Copy link
Copy Markdown
Member Author

Landed as ef47687.

Next steps

  1. Submit PR for part about supporting deps for additional discussion
  2. Submit PRs for 6.X and 4.X as backport/tweak to be appropriate for those levels

mhdawson added a commit to mhdawson/io.js that referenced this pull request Mar 20, 2017
doc: add statement about support for dependencies
865ad24 
- 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.
mhdawson mentioned this pull request Mar 20, 2017
Closed
3 tasks

mhdawson commented Mar 20, 2017

Copy link
Copy Markdown
Member Author

PR for discussion around what we should say about supporting dependencies: #11942

thefourtheye commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

Closing this PR as the commit has landed in ef47687

mhdawson commented Mar 20, 2017

Copy link
Copy Markdown
Member Author

PR to add supported platform list to 6.X - #11943

jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
doc: add supported platforms list
c05bf36 
PR-URL: nodejs#11872
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
mhdawson added a commit to mhdawson/io.js that referenced this pull request Mar 21, 2017
doc: add supported platforms list
061a0d1 
PR-URL: nodejs#11872
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
jasnell pushed a commit that referenced this pull request Mar 22, 2017
doc: add supported platforms list
74a7a1d 
Original Commit Message:
  PR-URL: #11872
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Rich Trott <rtrott@gmail.com>
  Reviewed-By: Roman Reiss <me@silverwind.io>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Backport-Of: #11872
PR-URL: #11943
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 4, 2017
doc: add supported platforms list
f05982b 
Original Commit Message:
  PR-URL: #11872
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Rich Trott <rtrott@gmail.com>
  Reviewed-By: Roman Reiss <me@silverwind.io>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Backport-Of: #11872
PR-URL: #11943
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
mhdawson added a commit to mhdawson/io.js that referenced this pull request Apr 4, 2017
doc: add supported platforms list
3401a27 
Original Commit Message:
  PR-URL: nodejs#11872
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Rich Trott <rtrott@gmail.com>
  Reviewed-By: Roman Reiss <me@silverwind.io>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Backport-Of: nodejs#11872
PR-URL: nodejs#11943
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell mentioned this pull request Apr 4, 2017
Closed
refack mentioned this pull request Apr 9, 2017
Closed
2 tasks
mhdawson added a commit that referenced this pull request Apr 11, 2017
doc: add supported platforms list
1c09d61 
Original Commit Message:
  PR-URL: #11872
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Rich Trott <rtrott@gmail.com>
  Reviewed-By: Roman Reiss <me@silverwind.io>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Backport-Of: #11872
PR-URL: #11943
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 17, 2017
doc: add supported platforms list
1546863 
Original Commit Message:
  PR-URL: #11872
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Rich Trott <rtrott@gmail.com>
  Reviewed-By: Roman Reiss <me@silverwind.io>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Backport-Of: #11872
PR-URL: #11943
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
doc: add supported platforms list
b9766bd 
Original Commit Message:
  PR-URL: #11872
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Rich Trott <rtrott@gmail.com>
  Reviewed-By: Roman Reiss <me@silverwind.io>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Backport-Of: #11872
PR-URL: #11943
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
mhdawson deleted the docsupport branch June 28, 2017 19:22
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
doc: add supported platforms list
5629e33 
Original Commit Message:
  PR-URL: nodejs/node#11872
  Reviewed-By: James M Snell <jasnell@gmail.com>
  Reviewed-By: Rich Trott <rtrott@gmail.com>
  Reviewed-By: Roman Reiss <me@silverwind.io>
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>

Backport-Of: nodejs/node#11872
PR-URL: nodejs/node#11943
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnoordhuis bnoordhuis approved these changes
jasnell jasnell approved these changes
Trott Trott approved these changes
+5 more reviewers
Fishrock123 Fishrock123 left review comments
targos targos left review comments
vsemozhetbyt vsemozhetbyt left review comments
silverwind silverwind approved these changes
gibfahn gibfahn approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

mhdawson

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL