Skip to content

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

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.

mhdawson added 6 commits April 4, 2017 14:41
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: nodejs#11943
  Reviewed-By: Ben Noordhuis <[email protected]>
  Reviewed-By: James M Snell <[email protected]>

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.

```

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 <[email protected]>
Reviewed-By: JoãReis <[email protected]>
Reviewed-By: Johan Bergströ[email protected]>
@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 <[email protected]>
Reviewed-By: JoãReis <[email protected]>
Reviewed-By: Johan Bergströ[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
PR-URL: #12091
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: JoãReis <[email protected]>
Reviewed-By: Johan Bergströ[email protected]>
@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