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

Remove re-export of is_same_file #56

Merged
merged 2 commits into from Jul 15, 2017

Conversation

Projects
None yet
4 participants
@jeremielate
Contributor

jeremielate commented Jun 22, 2017

Added a wrapper around this function that is deprecated.
Fixes #43.

Remove re-export of is_same_file
Added a wrapper around this function that is deprecated.
Fixes #43.
@KodrAus

Thanks @jeremielate! I've just left a comment about deprecating the pub use, but I wasn't sure if there was a reason you did it this way.

@@ -101,7 +101,12 @@ use std::path::{Path, PathBuf};
use std::result;
use std::vec;
pub use same_file::is_same_file;

This comment has been minimized.

@KodrAus

KodrAus Jun 26, 2017

Contributor

Should we be able to apply the deprecated attribute to this re-export? Is there a reason not to?

@KodrAus

KodrAus Jun 26, 2017

Contributor

Should we be able to apply the deprecated attribute to this re-export? Is there a reason not to?

@BurntSushi

This comment has been minimized.

Show comment
Hide comment
@BurntSushi

BurntSushi Jun 26, 2017

Owner

Since we're releasing a 2.0, I think we should just remove this API instead of deprecating it. Two reasons. Firstly, this is pretty niche and I doubt many are using it. Secondly, switching over to the same-file crate should be a trivial fix.

Owner

BurntSushi commented Jun 26, 2017

Since we're releasing a 2.0, I think we should just remove this API instead of deprecating it. Two reasons. Firstly, this is pretty niche and I doubt many are using it. Secondly, switching over to the same-file crate should be a trivial fix.

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jul 6, 2017

sgtm @jeremielate can you update the pr to remove the deprecated function?

brson commented Jul 6, 2017

sgtm @jeremielate can you update the pr to remove the deprecated function?

@KodrAus

KodrAus approved these changes Jul 6, 2017

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jul 15, 2017

This is ready for another look @BurntSushi

brson commented Jul 15, 2017

This is ready for another look @BurntSushi

@BurntSushi

This comment has been minimized.

Show comment
Hide comment
@BurntSushi
Owner

BurntSushi commented Jul 15, 2017

Thanks @jeremielate!

@BurntSushi BurntSushi merged commit 86238d6 into BurntSushi:master Jul 15, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment