-
Notifications
You must be signed in to change notification settings - Fork 0
Improved overflow support: Overflow auto-handling for file watches #25
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
Improved overflow support: Overflow auto-handling for file watches #25
Conversation
… file in `JDKFileWatch` (including tests)
…mproved-overflow-support/overflow-policies-for-file-watches
sungshik
left a comment
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.
Clarifying comment
| } | ||
| case PATH_ONLY: { | ||
| var result = new JDKFileWatch(path, executor, eventHandler); | ||
| var result = new JDKFileWatch(path, executor, h); |
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.
Missing context: h is the compound event handler that consists of: (a) the user-defined eventHandler; (b) the overflow auto-handler that generates synthetic events
…support/overflow-policies-for-file-watches
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## improved-overflow-support-main #25 +/- ##
==================================================================
+ Coverage 79.7% 80.6% +0.9%
- Complexity 107 111 +4
==================================================================
Files 14 14
Lines 483 486 +3
Branches 46 47 +1
==================================================================
+ Hits 385 392 +7
+ Misses 70 67 -3
+ Partials 28 27 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
DavyLandman
left a comment
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.
LGTM, just 1 improvement for the test I would like to propose
This PR addresses this comment: in
JDKFileWatch, overflow events should be propagated from the parent directory being "physically" watched to the file being "logically" watched.The general auto-handling of overflows immediately works as-is, as shown by the tests (added in this PR as well).