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

Stop reexporting Errno variants #696

Merged
merged 1 commit into from
Dec 5, 2017
Merged

Stop reexporting Errno variants #696

merged 1 commit into from
Dec 5, 2017

Conversation

jonas-schievink
Copy link
Contributor

@jonas-schievink jonas-schievink commented Jul 20, 2017

Closes #664 (unsure if this is everything needed)

Copy link
Contributor

@Susurrus Susurrus left a comment

Choose a reason for hiding this comment

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

This also needs a changelog entry under "Removed" that states that the errno constants were removed from a specific path.

}

/// Create a new invalid argument error (`EINVAL`)
pub fn invalid_argument() -> Error {
Error::Sys(errno::EINVAL)
Error::Sys(Errno::EINVAL)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not certain that I like Errno::EINVAL and Errno::last(). I don't think it makes sense for last() to be in the same place in the heirarchy as EINVAL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure about that? This is definitely the most "rustic" way to do it.

@Susurrus
Copy link
Contributor

Susurrus commented Dec 3, 2017

This still actually has Errno visible from the root when I think it should stay within nix::errno. Can you fix that?

@Susurrus
Copy link
Contributor

Susurrus commented Dec 4, 2017

LGTM.

@asomers You want to give this a lookover at all?

@Susurrus
Copy link
Contributor

Susurrus commented Dec 4, 2017

@jonas-schievink if you could squash this into a single commit, I'll go ahead and merge this. Thanks!

cc #664 (unsure if this is everything needed)
@Susurrus
Copy link
Contributor

Susurrus commented Dec 5, 2017

Perfect, thanks @jonas-schievink!

bors r+

bors bot added a commit that referenced this pull request Dec 5, 2017
696: Stop reexporting `Errno` variants r=Susurrus a=jonas-schievink

Closes #664 (unsure if this is everything needed)
@bors
Copy link
Contributor

bors bot commented Dec 5, 2017

@bors bors bot merged commit 512c351 into nix-rust:master Dec 5, 2017
@jonas-schievink jonas-schievink deleted the reexport branch December 5, 2017 08:18
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.

2 participants