[v12.x] backport #27224 as semver-patch#27483
Closed
BridgeAR wants to merge 2 commits intonodejs:v12.x-stagingfrom
Closed
[v12.x] backport #27224 as semver-patch#27483BridgeAR wants to merge 2 commits intonodejs:v12.x-stagingfrom
BridgeAR wants to merge 2 commits intonodejs:v12.x-stagingfrom
Conversation
This caches the current working directory and only updates the variable if `process.chdir()` is called. PR-URL: nodejs#27224 Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
This makes sure nodejs#27224 is possible to being backported in a semver-patch way.
Collaborator
nodejs-github-bot
added
process
addaleax
approved these changes
Apr 29, 2019
BridgeAR
added
the
author ready
Collaborator
targos
approved these changes
Apr 30, 2019
BethGriggs
approved these changes
Apr 30, 2019
joyeecheung
approved these changes
May 1, 2019
| const originalCwd = process.cwd; | ||
|
|
||
| process.cwd = function() { | ||
| cachedCwd = originalCwd(); |
Member
There was a problem hiding this comment.
I don't think we have a proper convention around this, but I'd add a comment explaining this is only for v12.x to reduce conflicts - this is where the difference between the two branches lie so it'll create conflicts when future commits touch here anyway.
Member
Author
There was a problem hiding this comment.
I struggle finding brief words describing this. I could just add // Backwards compatibility patch: removed code to prevent breaking change..
I personally do not think it would be bad without the comment, since we just work on master and people backporting code should see the compatibility commit.
| } | ||
|
|
||
| function cwd() { | ||
| cachedCwd = binding.cwd(); |
Member
There was a problem hiding this comment.
Similarly I'd add a comment here.
codebytere
approved these changes
May 1, 2019
Member
targos
pushed a commit
that referenced
this pull request
May 5, 2019
This caches the current working directory and only updates the variable if `process.chdir()` is called. PR-URL: #27224 Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com> Backport-PR-URL: #27483
targos
pushed a commit
that referenced
this pull request
May 5, 2019
This makes sure #27224 is possible to being backported in a semver-patch way. PR-URL: #27483 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Merged
This was referenced May 7, 2019
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The actual breaking changes in #27224 have been reverted here in a way that most code was possible to being backported.
This is necessary to reduce conflicts in our release lines. #27224 was the first semver-major to land after the v12 cut-off and thus it should also be the first to be backported. This is a trial run since without such backports it becomes increasingly difficult to put together releases after a while.
// cc @nodejs/releasers
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes