-
Notifications
You must be signed in to change notification settings - Fork 76
Move files, rename classes: "MacOS" watcher is used on Windows too. #2274
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
Conversation
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
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.
Code Review
This pull request is a great refactoring effort to deduplicate the watcher logic for MacOS and Windows into a unified RecursiveDirectoryWatcher. The file structure is now cleaner and more logical. I've found a couple of issues: a minor typo in a comment and a more significant issue regarding the default behavior for MacOS watchers, which now run in an isolate. My review includes suggestions to address these points.
pkgs/watcher/lib/src/directory_watcher/recursive/recursive_directory_watcher.dart
Outdated
Show resolved
Hide resolved
pkgs/watcher/lib/src/directory_watcher/recursive/recursive_directory_watcher.dart
Outdated
Show resolved
Hide resolved
06339a4 to
4cdc2b6
Compare
PR HealthLicense Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with
Breaking changes
|
| Package | Change | Current Version | New Version | Needed Version | Looking good? |
|---|---|---|---|---|---|
| watcher | Non-Breaking | 1.1.4 | 1.1.5-wip | 1.2.0 Got "1.1.5-wip" expected >= "1.2.0" (non-breaking changes) |
This check can be disabled by tagging the PR with skip-breaking-check.
Coverage ⚠️
| File | Coverage |
|---|---|
| pkgs/watcher/lib/src/directory_watcher.dart | 💔 44 % ⬇️ 11 % |
| pkgs/watcher/lib/src/directory_watcher/linux/linux_directory_watcher.dart | 💚 68 % |
| pkgs/watcher/lib/src/directory_watcher/polling/directory_list.dart | 💚 67 % |
| pkgs/watcher/lib/src/directory_watcher/polling/polling_directory_watcher.dart | 💚 78 % |
| pkgs/watcher/lib/src/directory_watcher/recursive/directory_tree.dart | 💔 Not covered |
| pkgs/watcher/lib/src/directory_watcher/recursive/event_tree.dart | 💔 Not covered |
| pkgs/watcher/lib/src/directory_watcher/recursive/isolate_recursive_directory_watcher.dart | 💔 Not covered |
| pkgs/watcher/lib/src/directory_watcher/recursive/recursive_directory_watcher.dart | 💔 Not covered |
| pkgs/watcher/lib/src/directory_watcher/recursive/recursive_native_watch.dart | 💔 Not covered |
| pkgs/watcher/lib/src/directory_watcher/recursive/watched_directory_tree.dart | 💔 Not covered |
| pkgs/watcher/lib/watcher.dart | 💔 0 % ⬇️ NaN % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
4cdc2b6 to
655b8aa
Compare
The MacOS and Windows outer watcher classes were identical, dedupe, move files and classes for this watcher into "recursive" subdirectory.
Similarly for Linux move files/classes to "linux" subdirectory, and for polling to new "polling" subdirectory.