-
Notifications
You must be signed in to change notification settings - Fork 9
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
Drop node 6,8 support and add 10,12 #3
base: master
Are you sure you want to change the base?
Conversation
SparshithNR
commented
Dec 17, 2019
- current LTS are node 10 and 12
- node 8 is reaching EOL
test.js
Outdated
@@ -77,7 +77,7 @@ describe("FSUpdater", () => { | |||
}); | |||
|
|||
it("must not be a symlink", () => { | |||
fs.symlinkSync(".", "tmp/out"); | |||
fs.symlinkSync(".", "tmp/out", "dir"); |
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.
"dir"
seems un-needed, I think node auto-detects.
@@ -77,7 +77,7 @@ describe("FSUpdater", () => { | |||
}); | |||
|
|||
it("must not be a symlink", () => { |
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.
Maybe we can feature detect the failure case, by doing try/catch
+ test scenario, if that fails, we can simply switch to use junction here. If we go down this path, leave a comment describing when/how etc this work-around can be removed.
@SparshithNR windows is still unhappy (you likely know) |
Yeah. using junction didn't resolve the issue. |