-
Notifications
You must be signed in to change notification settings - Fork 67
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
Rollup! #10
Rollup! #10
Conversation
r+ for the |
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.
Looks good! 👍
@@ -99,5 +115,5 @@ pub fn handle_dump(panic_info: &PanicInfo) -> Result<String, FailError> { | |||
} | |||
|
|||
let report = Report::new(Method::Panic, expl); | |||
return report.persist(); | |||
report.persist() |
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.
I am a fan of explicit return
statements but I think I'm in the minority there in the Rust community 😜
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.
Who cares what the rust community thinks – clippy doesn't like it :D
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.
Al̶l̛ ̨hai̷l҉ th̵è įǹfa͝l҉lib̢le cĺi̴p͟p̵y̸
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.
Looks good to me!
@killercup as a side note: I don't usually tend to bump versions in a PR, but instead wait until after merge. Usually so it can be labeled & tagged in a separate commit / shows up in the commit log as standalone. Nothing to block this on though. |
@yoshuawuyts ah no, that's fine, I just saw your comment in #6 about the version bump and went ahead. Let me remove that commit here. We should probably go over the docs before a release anyway |
chore(config): migrate renovate config
Wow, there's a lot of stuff going on in this repo! This is a rollup of #6 and #9 -- and I'm pretty sure also of #8 of which #9 is pretty much a superset.
So, to summarize:
Path
/PathBuf
instead ofString
in a few places…and it finally it bumps the version to 0.3 because, even though
print_msg
now accepts strictly more inputs (you can still pass in aString
but now also give it aPath
orPathBuf
of&str
), the output type ofhandle_dump
changed :(