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

Update dependencies #162

Merged
merged 5 commits into from
Aug 29, 2018
Merged

Update dependencies #162

merged 5 commits into from
Aug 29, 2018

Conversation

dimbleby
Copy link
Contributor

I've updated a bunch of dependencies, in a series of commits.

Much the biggest was mio.

I don't have access to a mac, so fsevent is left as an exercise for the reader.

@dimbleby
Copy link
Contributor Author

NB spurious test failures with weird \u{1} and suchlike are hannobraun/inotify-rs#117.

@passcod
Copy link
Member

passcod commented Aug 25, 2018

Wow, good work! Thank you very much

I think this is alright, but I'll make CI run a few more times to get the spurious failures some extra coverage before merging (after the inotify fix is in).

@passcod
Copy link
Member

passcod commented Aug 25, 2018

Hmm, the windows test fails consistently (not just in this PR) on move_in_directory_watch_subdirectories... would you have any insight given you looked at that code? Does it fail when run locally, or is it one of those pesky "fails only in CI" things?

Also could you move inotify/mod.rs to inotify.rs given the flags file is gone now :)

@dimbleby
Copy link
Contributor Author

Windows tests failed for me consistently before I started - all of create_directory_watch_subdirectories, create_rename_overwrite_file and move_in_directory_watch_subdirectories fail locally from master. I'm afraid I haven't investigated, sorry.

I'll move the code around as you propose and push shortly.

I'd suggest not merging this - or certainly not publishing it - until a fix for that inotify bug is released; and then presumably we'll want to depend on version 0.6.1 rather than 0.6.0.

@passcod
Copy link
Member

passcod commented Aug 25, 2018

Certainly. I've subscribed to the inotify issue, too :)

@@ -34,7 +34,7 @@ fsevent = "^0.2.17"
fsevent-sys = "^0.1.3"

[target.'cfg(windows)'.dependencies]
winapi = "^0.2.5"
winapi = "^0.3.5"
kernel32-sys = "^0.2.1"

Choose a reason for hiding this comment

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

When upgrading winapi to "0.3", kernel32-sys should be dropped.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack, thanks

@hannobraun
Copy link

I've merged hannobraun/inotify-rs#117 and released inotify 0.6.1.

@dimbleby
Copy link
Contributor Author

@hannobraun thanks! I've updated this pull request to match

@passcod
Copy link
Member

passcod commented Aug 28, 2018

Thanks all! I'll merge and release tomorrow (for me). In ~12 hours

@passcod passcod merged commit 3431e2e into notify-rs:master Aug 29, 2018
@passcod
Copy link
Member

passcod commented Aug 29, 2018

Released in 4.0.5

@dimbleby dimbleby deleted the update-dependencies branch August 29, 2018 08:10
passcod added a commit that referenced this pull request Jan 22, 2019
This was added in #162 but I don't think it's actually needed. All events go through this, even shutdown events (so we don't need to handle signals ourselves / use `poll_interruptible`).

Reported in #173
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants