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

replace unwrap() with expect(format!(...)) #32

Merged
merged 1 commit into from
Nov 7, 2015

Conversation

durka
Copy link
Contributor

@durka durka commented Nov 7, 2015

unwrap() produces panics with no explanation, and on some platforms there are no line numbers in the backtrace even in debug builds (rust-lang/rust#24346). expect() with an informative message is a much better user experience.

before:

$ ag -Q 'unwrap(' src | wc -l
  15

after:

$ ag -Q 'unwrap(' src | wc -l
   0

I made my best guess for an explanation at each error location, but they should be audited by someone who knows how the code works!

@durka durka mentioned this pull request Nov 7, 2015
@johannhof
Copy link
Contributor

This looks useful in the short term, but most of these errors should be handled properly and converted to warnings instead.

For anything that is so critical that a crash is completely inevitable, we should create our own CobaltError and return that to the build function.

So +1 from me, but let's make an issue on revisiting expect/unwrap handling.

Thanks for the contribution ❤️

johannhof pushed a commit that referenced this pull request Nov 7, 2015
replace unwrap() with expect(format!(...))
@johannhof johannhof merged commit 63fffe6 into cobalt-org:master Nov 7, 2015
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.

2 participants