-
Notifications
You must be signed in to change notification settings - Fork 150
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
Allow --verbose, --debug and --allow-self-signed everywhere #411
Conversation
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 great. I have two general remarks :
- We should be careful to add these common_options to future commands when we add them : should we document this ?
- Do you think we should add some kind of functional testing on these options' positions ? I think we should at least write the exhaustive list of commands to QA (which may be super long...)
WDYT ?
I agree with the idea, but I don't know a reliable place to document it, any suggestions? On the other hand cargo-culting is likely to prevent issues :)
Testing all possible combinations sounds overkill. Maybe modifying our tests to use different positions in different tests would be good enough? |
bf5610b
to
cf0b12f
Compare
I agree with your second point :
Adding some variations in the functional tests looks wise. |
cf0b12f
to
65e424d
Compare
Codecov Report
@@ Coverage Diff @@
## main #411 +/- ##
==========================================
+ Coverage 94.27% 94.49% +0.21%
==========================================
Files 80 82 +2
Lines 3425 3521 +96
==========================================
+ Hits 3229 3327 +98
+ Misses 196 194 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
65e424d
to
a9e22b0
Compare
Just did this. Going to merge when CI is happy. |
Description
Click does not allow defining options across groups, this means
ggshield -v secret scan path foo.py
works but notggshield secret scan path foo.py -v
.What has been done
This PR introduces a new module in
cmd
:common_options
. This module provides anadd_common_options()
decorator to define Click options we want to make available anywhere on the command-line. Supported options are:-v
,--verbose
--debug
--allow-self-signed
With these changes, all these commands now work and do the same thing:
In the next
episodePRI have another branch stacked on top of this one to do something similar for
secret scan
options like--json
,--exit-zero
,--show-secrets
...Issue
Part of #197