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

remove confusing panic messages from examples #2218

Merged

Conversation

r-arias
Copy link

@r-arias r-arias commented Nov 11, 2019

The publish_post examples contained a confusing panic!() that
suggested the update statement would return an error, when a post to be
updated could not be found. In reality, however, that case results in an
Ok(0) value.

To avoid confusion, this commit replaces the panicking
unwrap_or_else() calls with a simpler unwrap().

@r-arias
Copy link
Author

r-arias commented Nov 11, 2019

woah, are all these failures my fault?

@weiznich
Copy link
Member

The ci is broken on beta because of rust-lang/rust#66295

@weiznich weiznich requested a review from a team November 11, 2019 22:14
@JohnTitor
Copy link
Member

Could you rebase this to fix the CI?

@r-arias r-arias force-pushed the fix/examples-update-unwrap-or-else branch from 23b4f02 to 4980c9b Compare December 7, 2019 09:40
@r-arias
Copy link
Author

r-arias commented Dec 7, 2019

Done, I think :)

.find(id)
.first(&connection)
.unwrap_or_else(|_| panic!("Unable to find post {}", id));
let post: Post = posts.find(id).first(&connection).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

The error message was correct in that case, or do I miss something?

Copy link
Author

Choose a reason for hiding this comment

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

uh, I might have been too quick there. let me look at it again.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, sorry! Good catch! 😳

The `publish_post` examples contained a confusing `panic!()` that
suggested the update statement would return an error, when a post to be
updated could not be found. In reality, however, that case results in an
`Ok(0)` value.

To avoid confusion, this commit replaces the panicking
`unwrap_or_else()` calls with a simpler `unwrap()`.
@r-arias r-arias force-pushed the fix/examples-update-unwrap-or-else branch from 4980c9b to 2e1f9e4 Compare December 7, 2019 20:22
@weiznich weiznich merged commit b525fb9 into diesel-rs:master Dec 11, 2019
@r-arias r-arias deleted the fix/examples-update-unwrap-or-else branch December 11, 2019 21:15
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.

3 participants