Skip to content

don't use Result::ok just to be able to use unwrap/unwrap_or - #23512

Merged
bors merged 1 commit into
rust-lang:masterfrom
oli-obk:result_ok_unwrap
Mar 21, 2015
Merged

don't use Result::ok just to be able to use unwrap/unwrap_or#23512
bors merged 1 commit into
rust-lang:masterfrom
oli-obk:result_ok_unwrap

Conversation

@oli-obk

@oli-obkoli-obk commented Mar 19, 2015

Copy link
Copy Markdown
Contributor

because then the call to unwrap() will not print the error object.

@rust-highfive

rust-highfive commented Mar 19, 2015

Copy link
Copy Markdown
Contributor

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

oli-obk
oli-obk reviewed Mar 19, 2015
View reviewed changes
Comment threadsrc/libstd/sync/mpsc/mod.rs Outdated

@oli-obkoli-obkMar 19, 2015

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

this is really ugly, but it represents the comment more.
maybe I should just leave the ok().unwrap()?

@alexcrichtonalexcrichtonMar 19, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I'd just leave this as it was.

oli-obk
oli-obk reviewed Mar 19, 2015
View reviewed changes
Comment threadsrc/test/run-pass/spawn2.rs

@oli-obkoli-obkMar 19, 2015

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

join() returns a Result<T, Box<Any + Send + 'static>>

there are lots of other cases where the result is ignored since Debug cannot be implemented generically to return "thread panicked" on Err

@oli-obk
oli-obkforce-pushed the result_ok_unwrap branch 4 times, most recently from ee74f02 to da18d83CompareMarch 19, 2015 16:00
@alexcrichton

alexcrichton commented Mar 19, 2015

Copy link
Copy Markdown
Member

Looks good to me, thanks! r=me with a squash and a nit

@oli-obk

oli-obk commented Mar 19, 2015

Copy link
Copy Markdown
ContributorAuthor

make check running, will squash once it's done

@oli-obk

oli-obk commented Mar 20, 2015

Copy link
Copy Markdown
ContributorAuthor

squashed and checked @alexcrichton

@alexcrichton

alexcrichton commented Mar 20, 2015

Copy link
Copy Markdown
Member

@bors: r+ b4a1e59

bors added a commit that referenced this pull request Mar 20, 2015
@bors
Auto merge of#23512- oli-obk:result_ok_unwrap, r=alexcrichton
e2fa53e
because then the call to `unwrap()` will not print the error object.
@bors

bors commented Mar 20, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit b4a1e59 with merge e2fa53e...

@bors

bors commented Mar 21, 2015

Copy link
Copy Markdown
Collaborator

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

@bors
bors merged commit b4a1e59 into rust-lang:masterMar 21, 2015
This was referenced Mar 21, 2015
Merged
Merged
@oli-obk
oli-obk deleted the result_ok_unwrap branch March 23, 2015 12:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@alexcrichtonalexcrichton

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@oli-obk@rust-highfive@alexcrichton@bors@pcwalton