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

src: process release lts property by Fishrock123 · Pull Request #16656 · nodejs/node · GitHub

/ node Public

src: process release lts property - #16656

Merged
Fishrock123 merged 2 commits into
nodejs:masterfrom
Fishrock123:process-release-lts-property
Nov 2, 2017
ConversationCommits (2)ChecksFiles changed
Merged

src: process release lts property#16656
Fishrock123 merged 2 commits into
nodejs:masterfrom
Fishrock123:process-release-lts-property

Conversation

Fishrock123 commented Oct 31, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

This is a forward-port of 455272a. It is required for LTS release branches to have the correct in-process information when configured with NODE_VERSION_IS_LTS and NODE_VERSION_LTS_CODENAME. It has no impact otherwise, other than a test which already covers up multiple possible name paths.

This should be backported to 8.x asap as it is missing there and as such process.release.lts is undefined in 8.9.0. Myself and @MylesBorins are hoping to get this into the security release later this week (from irc).

This lands cleanly on 8.x.

cc also @nodejs/release

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

test, src, lts

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

Fishrock123 added lib / src Issues and PRs related to general changes in the lib or src directory. lts Issues and PRs related to Long Term Support releases. labels Oct 31, 2017
nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 31, 2017

Fishrock123 commented Oct 31, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

(Reminding myself, the first commit has configurable misspelt.)

MylesBorins approved these changes Oct 31, 2017
View reviewed changes

MylesBorins left a comment

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

LGTM

Were we going to include updates to node_version.h to make sure the LTS related flags are in there?

Comment thread doc/api/process.md Outdated

MylesBorins Oct 31, 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

is there an indent change here or is just github rendering weirdly?

gibfahn Nov 1, 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

Yep, looks like an indent.

Fishrock123 force-pushed the process-release-lts-property branch from a84754a to a808d96 Compare November 1, 2017 18:24
Fishrock123 force-pushed the process-release-lts-property branch from a808d96 to 10fbcef Compare November 1, 2017 18:26
gibfahn mentioned this pull request Nov 2, 2017
Closed
Fishrock123 force-pushed the process-release-lts-property branch from 10fbcef to 7f73d81 Compare November 2, 2017 16:14
rvagg and others added 2 commits November 2, 2017 09:18
src: add 'dynamic' process.release.lts property
905cd43 
This makes the process.release.lts property configurable by a constant.

This ref is the original PR to v6.x.
Refs: nodejs#3212

 Conflicts:
        doc/api/process.md

PR-URL: nodejs#16656
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
test: update process-release for Node 8 Carbon
d4471e0 
PR-URL: nodejs#16656
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Fishrock123 force-pushed the process-release-lts-property branch from 7f73d81 to d4471e0 Compare November 2, 2017 16:26
Fishrock123 merged commit d4471e0 into nodejs:master Nov 2, 2017
Fishrock123 deleted the process-release-lts-property branch November 2, 2017 16:27
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Nov 2, 2017
src: add 'dynamic' process.release.lts property
bf26b96 
This makes the process.release.lts property configurable by a constant.

This ref is the original PR to v6.x.
Refs: nodejs#3212

 Conflicts:
        doc/api/process.md

PR-URL: nodejs#16656
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Nov 2, 2017
test: update process-release for Node 8 Carbon
dfac6cc 
PR-URL: nodejs#16656
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

Fishrock123 commented Nov 2, 2017

Copy link
Copy Markdown
Contributor Author

Thanks. landed also on v8.x-staging as bf26b96 and dfac6cc.

I will follow up with a PR that adds default properties to node_version.h.

gibfahn mentioned this pull request Nov 5, 2017
Merged
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 6, 2017
src: add 'dynamic' process.release.lts property
c40b3c6 
This makes the process.release.lts property configurable by a constant.

This ref is the original PR to v6.x.
Refs: nodejs#3212

 Conflicts:
        doc/api/process.md

PR-URL: nodejs#16656
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 6, 2017
test: update process-release for Node 8 Carbon
d178c6d 
PR-URL: nodejs#16656
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
cjihrig mentioned this pull request Nov 6, 2017
Merged
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

cjihrig cjihrig approved these changes
gireeshpunathil gireeshpunathil approved these changes
+3 more reviewers
gibfahn gibfahn left review comments
apapirovski apapirovski approved these changes
MylesBorins MylesBorins approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. lts Issues and PRs related to Long Term Support releases.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL