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

Drop node 6,8 support and add 10,12 #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SparshithNR
Copy link

  • current LTS are node 10 and 12
  • node 8 is reaching EOL

@SparshithNR
Copy link
Author

nodejs/node#31231

test.js Outdated Show resolved Hide resolved
test.js Outdated
@@ -77,7 +77,7 @@ describe("FSUpdater", () => {
});

it("must not be a symlink", () => {
fs.symlinkSync(".", "tmp/out");
fs.symlinkSync(".", "tmp/out", "dir");

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", () => {

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.

@stefanpenner
Copy link

@SparshithNR windows is still unhappy (you likely know)

@SparshithNR
Copy link
Author

@SparshithNR windows is still unhappy (you likely know)

Yeah. using junction didn't resolve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants