-
Notifications
You must be signed in to change notification settings - Fork 93
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
Also listened to 'add' event for chokidar file watchers #162
Conversation
Err. CI is complaining about lines I didn't modify... |
index.js
Outdated
}) | ||
.on('change', file => { | ||
let recompile = [] | ||
const onAddOrChange = file => { |
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.
const onAddOrChange
=> const recompile
index.js
Outdated
.on('change', file => { | ||
let recompile = [] | ||
const onAddOrChange = file => { | ||
let recompile = [] |
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.
let recompiled
=> let changed
Travis is definitely unrelated (something weird with @RyanZim Does this require changes for |
Ignore my previous deleted comment. Will fix prettier errors ASAP. |
Fixed, please rebase on |
To be honest, I'm no longer using this tool, and development doesn't work on my OS (#164). I'm not going to have the time to investigate the single CI test failure. Hopefully this PR ends up being of some use to you! (Edit: unsubscribed; ping me if you need anything!) |
Could you nevertheless rebase, so this can be merged ? Are 'only' the current tests failing on windows or doesn't it work at all (normal CLI usage) ? |
Moves the 'change' callback to a separate `recompile` that now is also passed to 'add'.
On Windows, none of the tests work when I run Out of curiosity, why the requirement to rebase? Do you not want PRs to squash commits? |
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.
@JoshuaKGoldberg Thx
The watch test are definitely flaky |
Yeah, they drive me nuts. Gotta figure out something sometime. |
@JoshuaKGoldberg I always squash commits; I just wanted to see the CI rerun with the prettier issue fixed. NVM the failing watch tests; they are unpredictably flacky. |
Finally got the CI to pass after restarting it for the nth time. 😬 Gotta do some quick tests locally before I merge. |
This PR doesn't work for me; since we're passing the de-globbed file list into chokidar. It's possible to fix this, but I've got to move carefully to ensure I don't break my smart recompile logic. |
I've stopped using the tool and don't really have time for this PR. Best of luck! :) |
Moves the 'change' callback to a separate onAddOrChange that now is also passed to 'add'.
Fixes #161.