Apply some of the cargo clippy suggestions
#649
Conversation
nyurik
force-pushed
the
linting
branch
2 times, most recently
from
77ab8ac to
0e4e715
Compare
| @@ -21,7 +21,7 @@ impl Mapping { | |||
| pub fn new(path: &Path) -> Option<Mapping> { | |||
| let map = super::mmap(path)?; | |||
| Mapping::mk_or_other(map, |map, stash| { | |||
| let object = Object::parse(&map)?; | |||
| let object = Object::parse(map)?; | |||
There was a problem hiding this comment.
mk_or_other is such a weird function...
src/symbolize/gimli.rs
Outdated
| if let Some(name) = id.xcoff_name() { | ||
| let data = object.section(stash, name).unwrap_or(&[]); | ||
| Ok(EndianSlice::new(data, Endian)) | ||
| } else { | ||
| Ok(EndianSlice::new(&[], Endian)) | ||
| } |
There was a problem hiding this comment.
I would somewhat prefer this stay as-is because "all the AIX code in this fn is at its own indentation" is on-purpose. I didn't specifically say so it was when I accepted this code, but I picked the "rightward drift" and other stylistic nits several times and this was not an oversight.
If you would prefer refactoring this in a different way that appeases the lint and keeps the current visible divide between "AIX" and "everyone else" then that's fine too.
There was a problem hiding this comment.
ok, fixed it with two sections - clear separation, plus IDE highlights which one is active
|
thx for the review, will fix it shortly... |
|
I would not consider changing PrintFmt to use the actual |
|
Note that asserting stacktrace in the unit test is kinda hacky -- |
Per workingjubilee's [comment](#649 (comment))
|
The test being hacky is fine, all tests are allowed to be hacky forever and ever as long as the test is correctly testing at least what it is supposed to test. However something merge conflicted with this, sorry about that. |
|
@workingjubilee fixed |
As @workingjubilee mentioned,
no one is really paying for professional maintenance of backtrace-rs, so might as well try to do some volunteer upkeep. Just to keep things tidy.Note that the code does not seem to have had
cargo fmtrun in a while, but I don't want to do it as part of code-changing PR.The
enum backtrace::print::PrintFmt {}seem to be requiring#[non_exhaustive], but that may result in some breaking changes? Not certain how this should be handled, and probably in a separate PR.