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

Use a consistent return type for OsStr methods #52

Closed
wants to merge 1 commit into from
Closed

Use a consistent return type for OsStr methods #52

wants to merge 1 commit into from

Conversation

dylni
Copy link
Contributor

@dylni dylni commented Apr 23, 2020

This could be a good change to make before a 1.0 release.

It makes the return types consistent for into_os_string, into_path_buf, and into_string. They already match for ByteSlice.

@BurntSushi
Copy link
Owner

Yeah, hmmm, this LGTM. I'll merge this once I get to the 1.0 release.

I'm not quite sure why I didn't do this originally. It looks like i went out of my way to avoid it. 🤔

@dylni
Copy link
Contributor Author

dylni commented Apr 23, 2020

Great. I was wondering if there was a reason for this I wasn't seeing.

@BurntSushi
Copy link
Owner

Fixing this is a breaking change, so I'm putting this in the 1.0 milestone.

@BurntSushi BurntSushi added this to the 1.0 release milestone May 10, 2020
BurntSushi pushed a commit that referenced this pull request Jul 5, 2022
Previously, they would just return 'Vec<u8>' as the error type. But a
'FromUtf8Error' gives strictly more information, so we return that
instead.

Closes #52
BurntSushi pushed a commit that referenced this pull request Jul 5, 2022
Previously, they would just return 'Vec<u8>' as the error type. But a
'FromUtf8Error' gives strictly more information, so we return that
instead.

Closes #52
@BurntSushi BurntSushi closed this in cf5a68c Jul 6, 2022
@dylni
Copy link
Contributor Author

dylni commented Jul 13, 2022

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants