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

doc: update required compiler level for AIX #20153

Closed
wants to merge 1 commit into from

Conversation

mhdawson
Copy link
Member

Compilers were updated for 10.X as per
discussion in: nodejs/build#925

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

Compilers were updated for 10.X as per
discussion in: nodejs/build#925
@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. labels Apr 19, 2018
@mhdawson
Copy link
Member Author

mhdawson commented Apr 19, 2018

@gibfahn
@jasnell this should be part of 10.X sorry for getting it in so late.

@mhdawson mhdawson requested a review from gibfahn April 19, 2018 15:02
@gireeshpunathil
Copy link
Member

should we change UNIX (the platform mentioned above) to be something else, as we now have introduced a new section for AIX ? else it may give readers an impression on AIX not a member of UNIX family - just a thought.

@mhdawson mhdawson mentioned this pull request Apr 19, 2018
@mhdawson
Copy link
Member Author

Any objections to fast tracking this, need to get it into 10.X. @nodejs/tsc

@mhdawson
Copy link
Member Author

mhdawson commented Apr 19, 2018

@gireeshpunathil I'm not too worried about having Unix and AIX listed as they are but open to other suggestions. We do want to get this sorted and into 10.X though so we don't have a lot of time to bikeshed.

@jasnell jasnell added this to the 10.0.0 milestone Apr 19, 2018
@jasnell jasnell added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 19, 2018
@gibfahn gibfahn added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 19, 2018
@gibfahn
Copy link
Member

gibfahn commented Apr 19, 2018

I'm fine with this because it's semver-major so we want to get it in 10.0.0, but I would otherwise say we should have a libstdc++ requirement in the table alongside the glibc requirement.

The supported toolchains is mostly talking about what you need to build node, not run it, so it's not obvious from this that it's actually required to run Node.

However the proper place for that information is in nodejs.org anyway (see nodejs/nodejs.org#955) so that's a separate issue.

@mhdawson
Copy link
Member Author

Going to land as I have 2 TSC +1's and no objection to fast track.

CI Run (lite since its doc only): https://ci.nodejs.org/job/node-test-pull-request-lite/553/

@mhdawson
Copy link
Member Author

CI green landing.

@mhdawson
Copy link
Member Author

Landed as 9459656

@mhdawson mhdawson closed this Apr 19, 2018
mhdawson added a commit that referenced this pull request Apr 19, 2018
Compilers were updated for 10.X as per
discussion in: nodejs/build#925

PR-URL: #20153
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@Trott
Copy link
Member

Trott commented Apr 19, 2018

Going to land as I have 2 TSC +1's and no objection to fast track.

I get it, there were time constraints, it's a doc text change, etc., but just so no one else gets the wrong idea: This is not how our fast-tracking process works. "no objections to fast-tracking" is not a sign of approval. Two Collaborator approvals to fast-tracking are required to fast-track a PR. [EDIT: This was landed about a half hour before a 10.0.0 deadline for commits to land, so again: I totally get it. It had to land.]

https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#waiting-for-approvals

Almost no one actually respects that rule [EDIT: including me from time to time, I'm sorry to say, although I've stopped, honest!], and we should consider changing it or else starting to energetically enforce it/point out violations. It's kind of ridiculous, and if one part of our procedures is routinely ignored, it encourages people to ignore other parts of our procedures.

jasnell pushed a commit that referenced this pull request Apr 19, 2018
Compilers were updated for 10.X as per
discussion in: nodejs/build#925

PR-URL: #20153
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@ofrobots
Copy link
Contributor

Post-landing LGTM.

@mhdawson mhdawson deleted the aix-compiler branch September 30, 2019 13:15
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. fast-track PRs that do not need to wait for 48 hours to land. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants