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

onWritten is not defined #298

Closed
jonschlinkert opened this issue May 1, 2018 · 6 comments · May be fixed by ajesse11x/gulp#4
Closed

onWritten is not defined #298

jonschlinkert opened this issue May 1, 2018 · 6 comments · May be fixed by ajesse11x/gulp#4

Comments

@jonschlinkert
Copy link

https://github.com/gulpjs/vinyl-fs/blob/master/lib/symlink/link-file.js#L30

@phated phated changed the title onWrite is not defined onWritten is not defined May 1, 2018
@phated
Copy link
Member

phated commented May 1, 2018

ping @erikkemperman - I'm not sure which callback it's supposed to call there. I believe this was pulled from another chunk of code by erik

@phated
Copy link
Member

phated commented May 1, 2018

@jonschlinkert
Copy link
Author

fwiw it didn't cause me any problems, I didn't have time to write more earlier. I just noticed it in the code and wanted to let you know.

As much as we all like to act like code snobs about stuff like this, it happens to everyone. Linters are still run by humans lol.

@phated
Copy link
Member

phated commented May 2, 2018

No worries. Our linting setup needs a lot of work.

@erikkemperman
Copy link
Member

#299

Indeed, this looks like a sloppy/paste from dest to symlink on my part. Apologies, I do usually try to take note of lint output!

Also, this path is apparently not covered by the tests -- or we would have caught this obviously -- but I didn't have time to remedy that just now. For what it's worth, we get here only on Windows, when creating a symlink to a target which exists but can't be statted (as in permissions, I guess).

@erikkemperman
Copy link
Member

PS thanks @jonschlinkert for reporting this -- much better to address this before it actually burns anyone.

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 a pull request may close this issue.

3 participants