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

[Issue #34] - removing more instances of .unwrap() from documentation in favor of try!() #320

Closed
wants to merge 3 commits into from

Conversation

hiimtaylorjones
Copy link

As per #34, there were still a few instances of .unwrap() left in documentation. So, this (hopefully) covers some more ground towards eliminating these instances.

Please note that try!() is a new Rust idiom to me personally, but I think I was able to get the hang of it pretty quickly. There's definitely a chance I messed something up though!

@hiimtaylorjones hiimtaylorjones changed the title [Issue #34] - removing more instances of .unwrap() from documentation in favor of try [Issue #34] - removing more instances of .unwrap() from documentation in favor of try! May 10, 2016
@hiimtaylorjones hiimtaylorjones changed the title [Issue #34] - removing more instances of .unwrap() from documentation in favor of try! [Issue #34] - removing more instances of .unwrap() from documentation in favor of try!() May 10, 2016
@@ -23,7 +23,7 @@ use types::VarChar;
/// # use diesel::insert;
/// # let connection = establish_connection();
/// # insert(&NewUser { name: "Ha%%0r".into() }).into(users)
/// # .execute(&connection).unwrap();
/// # .try!(execute(&connection));
Copy link

Choose a reason for hiding this comment

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

Sadly, macros (which try! is) can't be called like methods, so the entire expression needs to be wrapped. You can try...

try!(insert(..).into(..).execute(..));

Copy link
Author

Choose a reason for hiding this comment

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

😢 Well that's a bummer.

Thanks for the pointer! Pushing up the change in a few.

@sgrif
Copy link
Member

sgrif commented May 10, 2016

I don't think any of these cases really need to be changed. We can't use try! for the most part in the main functions, what we're more looking for is anywhere that we're doing assert_eq!(foo, bar.unwrap()) where we could instead make a change like assert_eq!(Ok(foo), bar). I'm not sure if there are any of those cases left or not.

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.

None yet

3 participants