Conversation
Before this change, the `fancy` feature required the `derive` feature to compile. Despite this, `derive` was not listed as a required feature to use `fancy`. This meant that building with: `cargo build --no-default-features --features fancy` would result in a compilation error! Now `fancy` can be used without `derive` enabled, and another use of `thiserror` has been removed!
|
Not sure what the doc consistency check is about, @zkat ! It seems to generate this diff, deleting a section of the README if I run the command in the makefile, but I don't know if that's correct...
EDIT: It looks like this check was already failing on |
zkat
left a comment
zkat
left a comment
There was a problem hiding this comment.
So exciting! Thanks so much for taking the time!
src/eyreish/into_diagnostic.rs
Outdated
| write!(f, "{msg}") | ||
| } | ||
| } | ||
| impl Error for DiagnosticError {} |
There was a problem hiding this comment.
error(transparent) forwards the source method
There was a problem hiding this comment.
Ach! Good catch! I think my tests must have missed that by not using an error with a source... I'll fix that up now!
There was a problem hiding this comment.
Should be fixed now!
| /// [`chain()`](Report::chain). | ||
| pub fn root_cause(&self) -> &(dyn StdError + 'static) { | ||
| self.chain().last().unwrap() | ||
| self.chain().next_back().unwrap() |
There was a problem hiding this comment.
This was one of the clippy lints! Because that iterator is double-ended, you can just pop from the back to get the last element more efficiently!
Apparently calling .last() is wasteful in this case because it iterates through the whole iterator before it can return the last value (https://rust-lang.github.io/rust-clippy/stable/index.html#/next_back)
I hadn't thought about that before!
|
I suppose changing MSRV is probably not a SemVer patch change... I'll roll back my fancy-pants |
|
Trying to fix that README thing now too, since the CI bullied me and I'm looking for revenge (green checkmark...) |
|
CI successfully slain ⚔️ 😌 Should be ready for review again! |
|
Thanks a bunch for this! I'll roll a new release later today. Feel free to ping me if I forget :) |
|
Sounds like a great plan! That'll be great for unblocking the |
2 participants
Closes #435 and should just be a patch-level SemVer change!
Currently blocking the removal of
synfromkdl-rss dependency tree, but with this PR merged, that should become rather straightforward!I also found and fixed a bug leading to compilation errors in
ffe374c, some clippy lints, and reused some code in53c8330.I also added tests wherever I was replacing
ErrororDiagnosticderives with something hand-written, just to be sure I wasn't changing any behavior!