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: indicate the format of process.version #34872

Closed
wants to merge 1 commit into from
Closed

Conversation

dguo
Copy link
Contributor

@dguo dguo commented Aug 22, 2020

I needed to parse process.version and didn't know that it is prefixed with a v until I inspected the value.

As an aside, it seems inconsistent to me that process.version is prefixed with a v, but process.versions.node isn't. Would it be worth opening an issue to discuss that?

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

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. labels Aug 22, 2020
@dguo dguo marked this pull request as ready for review August 22, 2020 02:55
@Trott
Copy link
Member

Trott commented Aug 24, 2020

As an aside, it seems inconsistent to me that process.version is prefixed with a v, but process.versions.node isn't. Would it be worth opening an issue to discuss that?

I'd need to check, but I think that was intentional. process.version preceded process.versions and using the 'v' seemed like a mistake because almost nobody ever needs it and has to strip it out to deal with the version as a number. But changing process.version would cause too much ecosystem breakage.

@Trott
Copy link
Member

Trott commented Aug 24, 2020

I'd need to check, but I think that was intentional. process.version preceded process.versions and using the 'v' seemed like a mistake because almost nobody ever needs it and has to strip it out to deal with the version as a number. But changing process.version would cause too much ecosystem breakage.

If my recollection is correct, it may be worthwhile to have a discussion about documentation-deprecating process.version to point people at process.versions.node instead.

@Trott
Copy link
Member

Trott commented Aug 24, 2020

If my recollection is correct, it may be worthwhile to have a discussion about documentation-deprecating process.version to point people at process.versions.node instead.

Or that conversation might be made unnecessary by the change here. Maybe it's enough to direct people to the right thing.

@Trott
Copy link
Member

Trott commented Aug 24, 2020

Landed in 0dae5d9

@Trott Trott closed this Aug 24, 2020
Trott pushed a commit that referenced this pull request Aug 24, 2020
PR-URL: #34872
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Aug 25, 2020
PR-URL: #34872
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request Aug 25, 2020
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #34872
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #34872
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants