swellaby / rusty-hook

git hook manager, geared toward Rust projects
MIT License
205 stars 11 forks source link

Revert to getopts for argument parsing #169

Open Scandiravian opened 2 years ago

Scandiravian commented 2 years ago

Changes

Related Issues

I'd like to get some input on the trajectory of my work to revert back to getopts, so I can hopefully get it to an acceptable state in the next couple of days. Please let me know if there's any concerns or comments about the work so far :slightly_smiling_face:

I'm not entirely happy with the changes yet, as I think the main() function has become a bit too complex. There's also a .to_string call in the create_hook_files that I'd like to get rid of.

codecov[bot] commented 2 years ago

Codecov Report

Merging #169 (21c7e96) into master (f431408) will decrease coverage by 16.16%. The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##            master     #169       +/-   ##
============================================
- Coverage   100.00%   83.83%   -16.17%     
============================================
  Files            4        4               
  Lines          142      167       +25     
============================================
- Hits           142      140        -2     
- Misses           0       27       +27     
Impacted Files Coverage Δ
src/git.rs 64.28% <ø> (-35.72%) :arrow_down:
src/rusty_hook.rs 73.58% <0.00%> (-26.42%) :arrow_down:
src/hooks.rs 97.14% <100.00%> (-2.86%) :arrow_down:
src/config.rs 89.23% <0.00%> (-10.77%) :arrow_down:

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 f431408...21c7e96. Read the comment docs.

calebcartwright commented 2 years ago

Haven't really had a chance to go through this yet, but in case it's helpful you can take a look at what we had prior to the original switch from getopts to clap in https://github.com/swellaby/rusty-hook/blob/39fbca3b90c6f37bfdacd23bc4d25b9a0da554f0/src/main.rs

The only potential user interaction with the executable is rusty-hook init, and even that won't happen in cases where folks use the "easy button" approach of adding rusty-hook as a dev dependency. As such, I'm not super concerned with providing an extremely rich user experience; rusty-hook is primarily executed by git hook scripts users never see nor care about

calebcartwright commented 2 years ago

Apologies, I thought CI was failing outright here, but see it's coverage related. That's a somewhat surprising drop, do you think it's legitimate based on the diff or a specious tarpaulin and/or codecov issue?