search

rjeczalik / notify

File system event notification library on steroids.
MIT License
902 stars 128 forks source link

Some fixes regarding unwatching and rewatching a path #129

Closed imsodin closed 6 years ago

imsodin commented 7 years ago

This started in issue #124 and replaces the previously split up PRs. This is bundled as the fixed problems come up in the same tests, but are in separate commits to make each change-diff about a single problem. The commits should explain what is going on.

imsodin commented 6 years ago

@rjeczalik Any feedback?
I need this downstream in Syncthing as tests expose some of these bugs. It would be nice if I didn't have to deviate from this repo.

rjeczalik commented 6 years ago

@imsodin Changes look good to me, except the canonical part in FSEvents watcher. How about we can revert the deletion of it for now?

rjeczalik commented 6 years ago

And sorry it took so long 😇

imsodin commented 6 years ago

@rjeczalik Don't worry about the time - I am just glad to get feedback!

About the canonical stuff see my response to your comment on the code: https://github.com/rjeczalik/notify/pull/129#discussion_r142227064

imsodin commented 6 years ago

About the rebase: I just spotted and undid a changed comment that belongs to #128, everything else did not change.

rjeczalik commented 6 years ago

Thanks for the fixes and improvements @imsodin!

Zillode commented 6 years ago

👍