swellaby / rusty-hook

git hook manager, geared toward Rust projects

MIT License

206 stars 11 forks source link

Switch from 'getopts' to 'clap' #115

Closed Absolucy closed 3 years ago

Absolucy commented 3 years ago

Changes

Switches from the getopts crate to clap 3.0.
- This allows for simpler, easier to understand code, and an actual help menu (invalid arguments won't panic now, they'll actually provide a help menu)

Absolucy commented 3 years ago

This also provides the basis for passing git args to hooks - they're actually part of the arguments now, and are passed (although not used yet) to run()

codecov[bot] commented 3 years ago

Codecov Report

Merging #115 into master will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #115   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          138       138           
=========================================
  Hits           138       138

Impacted Files	Coverage Δ
src/git.rs	`100.00% <ø> (ø)`
src/rusty_hook.rs	`100.00% <ø> (ø)`

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 39fbca3...d0b2240. Read the comment docs.

calebcartwright commented 3 years ago

Thanks for the PR!

I'd originally gone with getopts over clap deliberately as the former was notably more lightweight (at least at the time), and the benefits clap provides around the user-friendliness front weren't really applicable for rusty-hook because we didn't anticipate much, if any, human interaction.

That being said I'm fine with the switch to clap especially given future plans, like #5, that'd involve the human more.

Absolucy commented 3 years ago

I'm not sure why shunit2.sh is failing shellcheck... considering that I haven't touched it in this PR

calebcartwright commented 3 years ago

I'm not sure why shunit2.sh is failing shellcheck... considering that I haven't touched it in this PR

That's unrelated to the changes here, no worries. That job is configured to use the latest version of ShellCheck so most likely what's happened is a new version that's change the way files are ignored. I'll update this later when I'm back at my machine and take a look at the changes here as well

Absolucy commented 3 years ago

Also, when multi-command support is added, perhaps a rusty-hook add command could be added?

calebcartwright commented 3 years ago

Also, when multi-command support is added, perhaps a rusty-hook add command could be added?

Could you elaborate? Not sure I follow

Absolucy commented 3 years ago

Like, rusty-hook add --hook pre-commit cargo check -- --all

calebcartwright commented 3 years ago

Like, rusty-hook add --hook pre-commit cargo check -- --all

Ahh okay gotcha. Probably worth opening a new issue for discussion. I'm not necessarily opposed to it but want to understand the use case/ease of use vs updating the toml file manually relative to additional code and features to support