Rework verbose mode #979
Rework verbose mode #979
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #979 +/- ##
==========================================
+ Coverage 91.58% 91.91% +0.32%
==========================================
Files 180 181 +1
Lines 7605 7593 -12
==========================================
+ Hits 6965 6979 +14
+ Misses 640 614 -26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
agateau-gg
force-pushed
the
agateau/use-ui-verbose
branch
from
1c59d19 to
b5e9685
Compare
|
Thank you for this MR. |
salome-voltz
left a comment
salome-voltz
left a comment
There was a problem hiding this comment.
I am a bit ocnfused about your use of ui.display_info for what was previously error logs (I haven't marked all of them though). In my understanding, you should use us.display_error or ui.display_warning instead. Am I missing something ? (this concerns the first commit)
| def is_verbose() -> bool: | ||
| """ | ||
| Convenient function to check if verbose messages are visible. Use this if displaying | ||
| verbose messages is costly (for example displaying a list of files) |
There was a problem hiding this comment.
| verbose messages is costly (for example displaying a list of files) | |
| verbose messages are costly (for example displaying a list of files) |
|
|
||
| config = ContextObj.get(ctx).config | ||
| # Update allow_self_signed in the config | ||
| # TODO: this should be reworked: if a command which writes the config is called with |
There was a problem hiding this comment.
| # TODO: this should be reworked: if a command which writes the config is called with | |
| # TODO: this should be reworked: if a command that writes the config is called with |
MY goal was to replace all calls to |
These functions are going to be useful for the next commit. This commit also provides a single way to reset the whole module for unit-tests: `ui.reset.reset()`.
Use ui.display_verbose() everywhere. Remove many `verbose: bool` arguments. Check if verbose is enabled using `ui.is_verbose()` instead of using `config.user_config.verbose`. This avoids the need to update `config.user_config.verbose` when called with `--verbose`, which is bad because if the command rewrites the configuration file, it's going to write `verbose: true` (this is still the case for `--allow-self-signed`).
agateau-gg
force-pushed
the
agateau/use-ui-verbose
branch
from
b5e9685 to
db00d97
Compare
What has been done
This PR reworks ggshield verbose mode. It does a few different things:
--debugautomatically enable verbose mode, so there is no need to add both-vand--debug.ui.display_verbose()instead ofverbosearguments from functions. They are no longer necessary because the UI now knows its verbosity level.Review
This PR is quite large, because verbose is everywhere. As usual, best reviewed commit by commit.
Validation
--debug: notice the verbose output is there