-
Notifications
You must be signed in to change notification settings - Fork 247
Dont change non path on win32 #107
Dont change non path on win32 #107
Conversation
src/variable.js
Outdated
@@ -1,14 +1,26 @@ | |||
import isWindows from 'is-windows' | |||
|
|||
const pathLikeEnvVarWhitelist = ['PATH', 'NODE_PATH'].reduce( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems overly complicated. What about simply new Set(['PATH', 'NODE_PATH'])
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL
Codecov Report
@@ Coverage Diff @@
## master #107 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 42 45 +3
=====================================
+ Hits 42 45 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great other than that one comment from @DanReyLop. Also, would you like to add yourself to the contributors table?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Thank you! I'm merging this into a branch called next
because this is a breaking change and I'd rather group this with other breaking changes we're considering. Thanks!
"profile": "https://twitter.com/paulcbetts", | ||
"contributions": [ | ||
"bug", | ||
"code" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed test
. You can add it in another PR if you want :)
@kentcdodds Works for me! |
* Add a whitelist for variables to : => ; ify * Plumb the name through * Write some tests * Dox, linter fascism * Much better! * Contributors BREAKING CHANGES: We no longer swap `:` for `;` on windows for any variables other than `PATH` and `NODE_PATH`.
Closes #106