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

Docsupport4x #12091

Closed
wants to merge 7 commits into from
Closed

Docsupport4x #12091

wants to merge 7 commits into from

Conversation

mhdawson
Copy link
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

First 2 commits are backports of the original changes to master and the updates for v6.x.

The last commit updates for the changes needed for 4.x

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. v4.x labels Mar 28, 2017
@mhdawson
Copy link
Member Author

@joaocgreis can you validate this is correct for Windows
@misterdjules can you validate it looks good for smartos
@jbergstroem can you review for the rest of the platforms

BUILDING.md Outdated
* [Visual Studio](https://www.visualstudio.com/) 2013 / 2015, all editions including the Community edition
* [Visual Studio](https://www.visualstudio.com/) Express 2013 / 2015 for Desktop
* [Visual Studio](https://www.visualstudio.com/) 2013, all editions including the Community edition
* [Visual Studio](https://www.visualstudio.com/) Express 2013 for Desktop
Copy link
Member

Choose a reason for hiding this comment

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

I believe this was better as it was, VS2015 is supported for building v4.x.

@mhdawson
Copy link
Member Author

@joaocgreis pushed commit to address comment.

@mhdawson
Copy link
Member Author

mhdawson commented Apr 3, 2017

@misterdjules @geek, does the smartOs part look right to you for 4.X ?

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 | x86, x64 | |

Choose a reason for hiding this comment

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

Why is it an = and not >=? In other words, do you mean that you think node v4.x doesn't run on SmartOS 15.x or 16.x images?

Copy link
Member Author

Choose a reason for hiding this comment

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

I probably changed that because of the way the build machines were setup. If it does run on 15, and 16 is should be >=, will update.

Choose a reason for hiding this comment

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

Thanks!

@misterdjules
Copy link

@mhdawson Thanks for the heads up! I left a question as a comment.

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>
Original Commit Message:
  PR-URL: nodejs#11943
  Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  Reviewed-By: James M Snell <jasnell@gmail.com>

Backport-Of: nodejs#11943
@mhdawson
Copy link
Member Author

mhdawson commented Apr 4, 2017

@misterdjules pushed commit to address your comment and rebased.

@misterdjules
Copy link

@mhdawson The SmartOS part looks good to me. Thanks again!

BUILDING.md Outdated
#### Unix

* GCC 4.8 or newer
* Clang 3.4 or newer
Copy link
Member

Choose a reason for hiding this comment

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

3.4.1 or newer.

@@ -53,6 +108,9 @@ $ make
$ [sudo] make install
```

Note that the above requires that `python` resolve to Python 2.6 or 2.7
and not a newer version.
Copy link
Member

Choose a reason for hiding this comment

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

Seems more clear to me to just point out that python 3.x isn't supported.

Copy link
Member

@jbergstroem jbergstroem left a comment

Choose a reason for hiding this comment

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

I think clang 3.4.1 is required; happy to let the python part pass if others feel its clear enough.

@mhdawson
Copy link
Member Author

mhdawson commented Apr 5, 2017

Since the versions we landed in master, and 6 describe the python dependency as it is shown lets leave it the same for consistency.

Will update to say clang 3.4.1 6.x will need to be update for that as well.

@mhdawson
Copy link
Member Author

mhdawson commented Apr 5, 2017

@jbergstroem pushed requested change

mhdawson added a commit that referenced this pull request Apr 11, 2017
PR-URL: #12091
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: Johan Bergströbugs@bergstroem.nu>
@mhdawson
Copy link
Member Author

landed in v4.x-staging as 918a26f

@mhdawson mhdawson closed this Apr 11, 2017
MylesBorins pushed a commit that referenced this pull request Apr 17, 2017
PR-URL: #12091
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: Johan Bergströbugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
PR-URL: #12091
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: JoãReis <reis@janeasystems.com>
Reviewed-By: Johan Bergströbugs@bergstroem.nu>
@MylesBorins MylesBorins mentioned this pull request Apr 19, 2017
@mhdawson mhdawson deleted the docsupport4x branch June 28, 2017 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants