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

src: add process.release.lts property & LTS CODENAME VOTING #3212

Closed
wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Oct 6, 2015

Looking for review of this against v4.x, and separately calling for votes from @nodejs/collaborators on what the codename for our first LTS line (v4.x) will be. We are aiming for v4.2.0 LTS to be released this Thursday so this has to be quick!

Adopting a codename for LTS releases is part of the original LTS plan and although we've had a couple of late-breaking objections to even having a codename we're going ahead with it anyway given that this has been baked into the LTS plan that has already been adopted. We'll give it a go for this first one and assess for v6 if we go ahead and do it again.

The objectives are something like this:

  • Something fun
  • Something to set the LTS releases apart from the standard releases, both technically (a process.release.lts property) and for non-code communication purposes. We have an additional burden of having to start an LTS at an arbitrary point of a semver-major + semver-minor, i.e. 4.2.0 in this case, being able to call it something from that point on sets it apart
  • Foster a shared culture around LTS as more than just another release line, having input on what the codename should be from the broader group also helps with a sense of shared ownership (also, fun again, imagine how boring Ubuntu would be without the fun names? It'd be as boring as Fedora has become)

The plan was for the @nodejs/lts group to come up with a short-list and call for voting. We've decided to go with the periodic-table for inspiration, there's a nice connection between the elements and the elemental nature of what we are trying to do with Node (i.e. small-core), also, there's lots to choose from! We also agreed that using alphabetic ordering will be helpful in deciphering order without needing additional context if people choose to refer to a line by its codename. These are what are available:

  • Actinium Ac
  • Aluminum Al
  • Americium Am
  • Antimony Sb
  • Argon Ar
  • Arsenic As
  • Astatine At

For various reasons we've had to trim that list right down and only have two candidates for the v4 LTS codename:

Actinium

Actinium is a silvery radioactive metallic element. Actinium glows in the dark due to its intense radioactivity with a blue light. Actinium was discovered in 1899 by André-Louis Debierne, a French chemist, who separated it from pitchblende. Friedrich Otto Giesel independently discovered actinium in 1902. The chemical behavior of actinium is similar to that of the rare earth lanthanum. The word actinium comes from the Greek aktis, aktinos, meaning beam or ray. Also, Actinium happens to have 4 isotopes, which is nice for our v4 release.

Argon

Argon was suspected to be present in air by Henry Cavendish in 1785 but wasn't discovered until 1894 by Lord Rayleigh and Sir William Ramsay. Argon is the third noble gas, in period 8, and it makes up about 1% of the Earth's atmosphere. Argon has approximately the same solubility as oxygen and it is 2.5 times as soluble in water as nitrogen . This chemically inert element is colorless and odorless in both its liquid and gaseous forms. It is not found in any compounds. This gas is isolated through liquid air fractionation since the atmosphere contains only 0.94% argon. The Martian atmosphere in contrast contains 1.6% of Ar-40 and 5 ppm Ar-36. World production exceeds 750.000 tonnes per year, the supply is virtually inexhaustible.

(Astatine might also have been in the running except for the slightly awkward pronunciation and this gem: "total world production of astatine to date is estimated to be less than a millionth of a gram, and virtually all of this has now decayed away.")

So, this vote is open to @nodejs/collaborators only, you get one vote each, just leave a comment with your choice below.

Also looking for reviews of the changes this introduces (assuming the codename will be inserted to the appropriate places when chosen).

@rvagg rvagg added the semver-minor PRs that contain new features and should be released in the next minor version. label Oct 6, 2015
@cjihrig
Copy link
Contributor

cjihrig commented Oct 6, 2015

Changes LGTM, once the real value is in there. My vote is for Argon because it's shorter, and easier to remember and say.

@evanlucas
Copy link
Contributor

+1 to Argon

@Trott
Copy link
Member

Trott commented Oct 6, 2015

Another vote for Argon
On Tue, Oct 6, 2015 at 6:07 AM Evan Lucas notifications@github.com wrote:

+1 to Argon


Reply to this email directly or view it on GitHub
#3212 (comment).

@mhdawson
Copy link
Member

mhdawson commented Oct 6, 2015

Changes lgtm once real value is there.

+1 to Argon

@@ -2736,6 +2736,9 @@ void SetupProcessObject(Environment* env,
Local<Object> release = Object::New(env->isolate());
READONLY_PROPERTY(process, "release", release);
READONLY_PROPERTY(release, "name", OneByteString(env->isolate(), "node"));
READONLY_PROPERTY(release,
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this make sure that lts is defined only for LTS releases?

Copy link
Member Author

Choose a reason for hiding this comment

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

this PR is against v4.x, that's how; I'm open to suggestions on doc changes that could go in to master if any

@aheckmann
Copy link
Contributor

+1 Argon

@targos
Copy link
Member

targos commented Oct 6, 2015

+1 for Argon, though I would have preferred if we picked the elements in the natural order and start with Hydrogen.

@thefourtheye
Copy link
Contributor

LGTM + Argon

@jasnell
Copy link
Member

jasnell commented Oct 6, 2015

+1 for Argon. Patch LGTM.

@mscdex mscdex added the process Issues and PRs related to the process subsystem. label Oct 6, 2015
@bnoordhuis
Copy link
Member

Can we make process.release.lts a number or number-as-string? Makes lexicographic comparison easy:

if (process.release.lts < 201709) {  // do this one thing
if (process.release.lts > 201510) {  // do this other thing

@ChALkeR
Copy link
Member

ChALkeR commented Oct 6, 2015

@rvagg What will the 10th LTS release be called?

@trevnorris
Copy link
Contributor

+1 to Ben's suggestion.

@Fishrock123
Copy link
Contributor

(we already bikeshedded that in the meeting)

Let's just do { name: 'Argon', date: 201510 }

@Trott
Copy link
Member

Trott commented Oct 7, 2015

Actinium Defeats Argon

@rvagg
Copy link
Member Author

rvagg commented Oct 7, 2015

@nodejs/lts opinions on @Fishrock123's suggestion above, my recollection is that we ended at the date not being all that helpful given that we already have process.version to tell us what version its running on. I'm -0 on making it a complex object.

I guess we could review the hangout video ... someone have the patience for that?

@Fishrock123
Copy link
Contributor

Let's just do { name: 'Argon', date: 201510 }

@nodejs/lts opinions on @Fishrock123's suggestion above, my recollection is that we ended at the date not being all that helpful given that we already have process.version to tell us what version its running on. I'm -0 on making it a complex object.

Actually, same. Going back to @zkat and @issacs's points, this is what the semver version is for. We only have an lts bit in to tell that it is actually an lts release and whether or not we make it a string, it might as well be a boolean.

@Fishrock123
Copy link
Contributor

Also, interesting thought: I guess this means we always have to bump a minor when going into an LTS phase?

@jbergstroem
Copy link
Member

Whatever name, just get @bnoordhuis's date parsing in there +1.

@Fishrock123
Copy link
Contributor

Whatever name, just get @bnoordhuis's date parsing in there +1.

As we bikeshedded in the LTS meeting, that's not actually useful. Just use the semver version instead (+ a check if it is an lts at all).

@rvagg rvagg mentioned this pull request Oct 7, 2015
8 tasks
@jbergstroem
Copy link
Member

@Fishrock123 haven't caught up on the bikeshedding just yet. Thanks for pointing me there though.

@Fishrock123
Copy link
Contributor

Also, fwiw, +1 Argon (my suggestion)(am I allowed to vote for my own?)

@rvagg
Copy link
Member Author

rvagg commented Oct 12, 2015

Updated PR to use Argon and use node_version.h macros as per @jasnell's proposal.

Also added in more strict testing so that it'll enforce no process.release.lts unless ~4.2.0 in which case it must be Argon.

@jasnell
Copy link
Member

jasnell commented Oct 12, 2015

LGTM. There are a few bits I'll fix up when it lands (i.e. "WHAT WILL THIS BE?" is the doc change)

rvagg added a commit that referenced this pull request Oct 12, 2015
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #3212
@jasnell
Copy link
Member

jasnell commented Oct 12, 2015

Landed in 42b936e

@jasnell jasnell closed this Oct 12, 2015
jasnell added a commit that referenced this pull request Oct 12, 2015
The first Node.js LTS release! See https://github.com/nodejs/LTS/
for details of the LTS process.

* **icu**: Updated to version 56 with significant performance improvements
  (Steven R. Loomis) #3281
* **node**:
  - Added new `-c` (or `--check`) command line argument for checking script
    syntax without executing the code (Dave Eddy) #2411
  - Added `process.versions.icu` to hold the current ICU library version
    (Evan Lucas) #3102
  - Added `process.release.lts` to hold the current LTS codename when the
    binary is from an active LTS release line (Rod Vagg) #3212
* **npm**: Upgraded to npm 2.14.7 from 2.14.4, see release notes:
  https://github.com/npm/npm/releases/tag/v2.14.7 for full details (Kat Marchán) #3299

PR-URL: #3258
jasnell added a commit that referenced this pull request Oct 12, 2015
The first Node.js LTS release! See https://github.com/nodejs/LTS/
for details of the LTS process.

* **icu**: Updated to version 56 with significant performance improvements
  (Steven R. Loomis) #3281
* **node**:
  - Added new `-c` (or `--check`) command line argument for checking script
    syntax without executing the code (Dave Eddy) #2411
  - Added `process.versions.icu` to hold the current ICU library version
    (Evan Lucas) #3102
  - Added `process.release.lts` to hold the current LTS codename when the
    binary is from an active LTS release line (Rod Vagg) #3212
* **npm**: Upgraded to npm 2.14.7 from 2.14.4, see release notes:
  https://github.com/npm/npm/releases/tag/v2.14.7 for full details (Kat Marchán) #3299

PR-URL: #3258
jasnell added a commit that referenced this pull request Oct 12, 2015
The first Node.js LTS release! See https://github.com/nodejs/LTS/
for details of the LTS process.

* **icu**: Updated to version 56 with significant performance improvements
  (Steven R. Loomis) #3281
* **node**:
  - Added new `-c` (or `--check`) command line argument for checking script
    syntax without executing the code (Dave Eddy) #2411
  - Added `process.versions.icu` to hold the current ICU library version
    (Evan Lucas) #3102
  - Added `process.release.lts` to hold the current LTS codename when the
    binary is from an active LTS release line (Rod Vagg) #3212
* **npm**: Upgraded to npm 2.14.7 from 2.14.4, see release notes:
  https://github.com/npm/npm/releases/tag/v2.14.7 for full details (Kat Marchán) #3299

PR-URL: #3258
jasnell added a commit that referenced this pull request Oct 12, 2015
The first Node.js LTS release! See https://github.com/nodejs/LTS/
for details of the LTS process.

* **icu**: Updated to version 56 with significant performance improvements
  (Steven R. Loomis) #3281
* **node**:
  - Added new `-c` (or `--check`) command line argument for checking script
    syntax without executing the code (Dave Eddy) #2411
  - Added `process.versions.icu` to hold the current ICU library version
    (Evan Lucas) #3102
  - Added `process.release.lts` to hold the current LTS codename when the
    binary is from an active LTS release line (Rod Vagg) #3212
* **npm**: Upgraded to npm 2.14.7 from 2.14.4, see release notes:
  https://github.com/npm/npm/releases/tag/v2.14.7 for full details (Kat Marchán) #3299

PR-URL: #3258
@rvagg rvagg deleted the process.release.lts branch October 13, 2015 00:24
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Nov 2, 2017
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 pushed a commit to Fishrock123/node that referenced this pull request Nov 2, 2017
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
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.