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

Replace Path with AsRef<Path> in Watcher functions #25

Merged
merged 6 commits into from
Jul 28, 2015
Merged

Replace Path with AsRef<Path> in Watcher functions #25

merged 6 commits into from
Jul 28, 2015

Conversation

frangio
Copy link
Contributor

@frangio frangio commented Jul 26, 2015

This should be a non-breaking change and it removes the need to call Path::new when calling watch or unwatch. This means watcher.watch("/tmp/foo") is now valid.

@passcod
Copy link
Member

passcod commented Jul 26, 2015

I've wondered how to do that... thank you very much for this :)
Add yourself to the Cargo.toml and I'll get this merged.

@frangio
Copy link
Contributor Author

frangio commented Jul 27, 2015

No problem :)

@frangio
Copy link
Contributor Author

frangio commented Jul 27, 2015

Hmm, I just realized that by removing the & now the methods are taking ownership of the argument when they really shouldn't. Give me a moment to sort it out.

@frangio frangio changed the title Replace &Path with AsRef<Path> in Watcher functions Replace Path with AsRef<Path> in Watcher functions Jul 27, 2015
@frangio
Copy link
Contributor Author

frangio commented Jul 27, 2015

That should be it.

@passcod
Copy link
Member

passcod commented Jul 27, 2015

Awesome :) I'll merge after work

passcod added a commit that referenced this pull request Jul 28, 2015
Replace Path with AsRef<Path> in Watcher functions
@passcod passcod merged commit 49a8539 into notify-rs:master Jul 28, 2015
@frangio frangio deleted the as_ref_path branch July 28, 2015 18:04
@frangio frangio restored the as_ref_path branch July 30, 2015 18:28
@frangio
Copy link
Contributor Author

frangio commented Jul 30, 2015

@passcod, sorry to bring this up again, but I think I was wrong with the last change I made (commit bb3c978). The method signature before that was better and is consistent with the Rust standard library. See for example http://doc.rust-lang.org/std/fs/struct.File.html#method.open. As for my concern regarding moving/borrowing, it's not a problem at all since the argument passed can still be a reference.

So I think commit bb3c978 should be reverted.

@passcod
Copy link
Member

passcod commented Aug 24, 2015

Done in 3340d74.

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

2 participants