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

Use bootignore patterns in watcher workers #663

Merged
merged 2 commits into from
Jun 20, 2018

Conversation

yannvanhalewyn
Copy link
Contributor

@yannvanhalewyn yannvanhalewyn commented Nov 23, 2017

Boot uses the ignore patterns parsed from .bootignore when syncing src and resource directories, but not in the underlying watch processes. This PR offers a start to solving this discrepancy.

It is also a potential solution for people have node-modules / figwheel builds et ceterae in resource directories experiencing FSEvent dropouts on Mac OSX (eg: #641) and improves performance in those cases - but mainly feels consistent.

note This won't exclude any added files after the watcher started, since the watchservices take dirs and don't have ignore options. It seems like that could be fixed in boot.file/watcher!. This also won't extend the watch task yet. It only will filter out .bootignore matches at register time when boot boots.

disclaimer I didn't test the entire build since make install fails with a TLS error. I just did not want to spend time debugging the build tool for debugging the build tool I was debugging. I did however test the adapted functions in a repl.

Copy link
Contributor

@arichiardi arichiardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes seem straightforward, I will try it out

@arichiardi
Copy link
Contributor

Thanks, seems sensible

@alandipert
Copy link
Contributor

This looks good, thank you for your contribution! However it needs a CHANGES.md entry.

@martinklepsch
Copy link
Member

@yannvanhalewyn Hey, would you mind adding an entry to CHANGES.md?

@martinklepsch
Copy link
Member

martinklepsch commented Jan 13, 2018

I didn't test the entire build since make install fails with a TLS error. I just did not want to spend time debugging the build tool for debugging the build tool I was debugging. I did however test the adapted functions in a repl.

@yannvanhalewyn could you check if you can still reproduce this and open an issue with the full exception/error? Thanks a lot in advance! I hope we can reduce these kinds of issues to make contributing easier.

PS. feel free to ignore my request about adding stuff to the changelog, I'll do it once I merge this.

@alandipert alandipert merged commit 59909f1 into boot-clj:master Jun 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants