jonhoo / inferno

A Rust port of FlameGraph
Other
1.68k stars 125 forks source link

Update from structopt to clap v3 #227

Closed Sciencentistguy closed 2 years ago

Sciencentistguy commented 2 years ago

Clap v3 has merged structopt directly into clap, and structopt itself is now in maintenance-only mode.

This PR switches all use of structopt over to clap's new #[derive(Parser)].

codecov[bot] commented 2 years ago

Codecov Report

Merging #227 (90ecf8e) into master (502c8f3) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #227   +/-   ##
=======================================
  Coverage   87.02%   87.02%           
=======================================
  Files          17       17           
  Lines        2381     2381           
=======================================
  Hits         2072     2072           
  Misses        309      309           

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 502c8f3...90ecf8e. Read the comment docs.

jonhoo commented 2 years ago

Thanks for taking the time to do this! It looks like this ends up bumping the MSRV, which I think is okay for this change, but we should update CI to match the new required version :+1: https://github.com/jonhoo/inferno/blob/502c8f33cb2dff8c08bf806f053a0d0a8659c8a6/azure-pipelines.yml#L5

jonhoo commented 2 years ago

Actually, before I merge, cc @djc for cargo-flamegraph to see how problematic this would be there. I think the move to clap3 is pretty worthwhile.

djc commented 2 years ago

Should be fine, we already ported flamegraph (which now disables the cli feature in inferno). I'm discussing the MSRV issue upstream.