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
[WIP] Expand error handling support #58
Conversation
alphabetize with new custom error name
impl FromStr for Bound { | ||
type Err = BoundParseError; | ||
fn from_str(s: &str) -> Result<Bound, BoundParseError> { | ||
type Err = NeverError; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requires feedback
Is this a necessary error? I renamed it to make it a more general error type, but I think that it can be eliminated altogether?
let outcome = t.test(&cfg); | ||
// TODO (@chrissimpkins): handle errors in this closure through least_satisfying | ||
// This line will still panic on calls to Toolchain.test | ||
let outcome = t.test(&cfg).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need help here
Site of a TODO for least_satisfying
function associated error handling
let outcome = t.test(&cfg); | ||
// TODO (@chrissimpkins): handle errors in this closure through least_satisfying | ||
// This line will still panic on calls to Toolchain.test | ||
let outcome = t.test(&cfg).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need help here
Site of a TODO for least_satisfying
function associated error handling
@@ -1398,10 +1368,10 @@ struct BisectionResult { | |||
|
|||
fn main() { | |||
if let Err(err) = run() { | |||
match err.downcast::<ExitError>() { | |||
Ok(ExitError(code)) => process::exit(code), | |||
match err.downcast::<ExitStatusError>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requires feedback
The ExitError
was in main and I simply renamed it to make it clear that this is the approach to explicitly define an exit status code number. It does not appear to be in use any longer. Maybe we just eliminate it?
Will rebase on the #57 merge if there is interest in this approach |
Is there any interest in supporting this? |
This PR refactors existing custom errors to a new
errors.rs
module and replaces panics with error handling support across all source files.WIP =>
Result
return types were added across all functions that raise errors; however, I need input on how to work the error handling through theleast_satisfying
closures in:cargo-bisect-rustc/src/main.rs
Lines 1249 to 1270 in 7ac0397
and
cargo-bisect-rustc/src/main.rs
Lines 1360 to 1383 in 7ac0397
At the moment, the errors raised in the Toolchain.test method will still panic during the
least_satisfying
tests.Fixes #16
Supersedes #48
TODO
least_satisfying
part ofbisect_nightlies
least_satisfying
part ofbisect_ci_between