Skip to content

Conversation

@KnapSac
Copy link
Contributor

@KnapSac KnapSac commented Oct 6, 2021

No description provided.

@codecov
Copy link

codecov bot commented Oct 6, 2021

Codecov Report

Merging #219 (a9e612c) into master (410018b) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #219      +/-   ##
==========================================
- Coverage   89.26%   89.23%   -0.03%     
==========================================
  Files          17       17              
  Lines        2356     2351       -5     
==========================================
- Hits         2103     2098       -5     
  Misses        253      253              
Impacted Files Coverage Δ
src/collapse/perf.rs 95.03% <100.00%> (-0.04%) ⬇️
src/collapse/vtune.rs 90.41% <100.00%> (ø)
src/flamegraph/attrs.rs 90.62% <100.00%> (ø)
src/flamegraph/color/palettes.rs 99.39% <100.00%> (-0.01%) ⬇️
src/flamegraph/mod.rs 97.25% <100.00%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 410018b...a9e612c. Read the comment docs.

This fixes warnings emitted using Rust 1.55.0, which were not yet
warnings in 1.52.0.
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@jonhoo jonhoo merged commit f62187c into jonhoo:master Oct 8, 2021
@KnapSac KnapSac deleted the update-rust-version branch October 8, 2021 06:33
@djc
Copy link
Contributor

djc commented Oct 12, 2021

I'm not sure why this bumped the MSRV version proactively? It didn't seem to be necessary? The small fixes here don't seem like a very good reason to force downstream code to a relatively fresh MSRV.

(You can use https://github.com/rust-lang/rust-clippy#specifying-the-minimum-supported-rust-version to have clippy skip lints that don't apply on your MSRV.)

@jonhoo
Copy link
Owner

jonhoo commented Oct 13, 2021

Ah, so, the bump was for #218 which is going to want to use split_once to avoid some slightly hair code. But I suppose you're right that it's not worth a MSRV bump yet, especially given that inferno is used by cargo-flamegraph. @KnapSac can you submit a PR that reverts the MSRV change? The clippy fixes I believe can all still remain, since strip_prefix was stabilized in 1.45.0.

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.

3 participants