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

Add an error type to FromStr impls #156

Merged
merged 2 commits into from May 24, 2017

Conversation

Projects
None yet
4 participants
@nivkner
Copy link
Contributor

nivkner commented May 20, 2017

Fixes #119

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented May 21, 2017

Awesome, thanks @nivkner! I think in light of #142 though we may wish to remove the Log prefix here perhaps?

@sfackler do you have thoughts on how to manage breaking changes like this? Do you think we should hold off on merging or maybe merge to a separate branch?

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented May 21, 2017

I would merge to master along with a note in Cargo.toml.

publish = false # this branch contains breaking changes
@nivkner

This comment has been minimized.

Copy link
Contributor Author

nivkner commented May 21, 2017

Removed the Log prefix as requested.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented May 22, 2017

Thanks @nivkner! We'll take it from here and figure out the best way to land this with other breaking changes to the log crate.

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented May 22, 2017

Seems fine to merge to master - we can always cut an 0.3.x branch if we need more releases from that version.

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented May 23, 2017

@nivkner any reason these messages are slightly different? I would prefer to pick one or the other.

@dtolnay dtolnay merged commit de11f39 into rust-lang-nursery:master May 24, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented May 24, 2017

Ah, this matches all of the existing error messages. I filed #166 to follow up.

@nivkner nivkner deleted the nivkner:parse-error branch Jun 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.