[FEATURE] Add verbose option #157
Conversation
Codecov Report
@@ Coverage Diff @@
## master #157 +/- ##
==========================================
+ Coverage 94.93% 94.97% +0.03%
==========================================
Files 18 18
Lines 731 736 +5
==========================================
+ Hits 694 699 +5
Misses 37 37
Continue to review full report at Codecov.
|
Irallia
requested review from
a team and
eaasna
and removed request for
a team
Irallia
force-pushed
the
FEATURE/add_verbose_option
branch
from
59a7804
to
4b0affd
Compare
There was a problem hiding this comment.
I'm not sure, if I should agree or disagree with this... So I share my thoughts:
As you see, the verbose variable is needed in every corner of the program and you have to change basically each and every function signature of the software. In my opinion, this is THE situation to use a public global variable. Your PR would shrink to a few lines, the future maintenance would be simpler, and you would avoid passing the boolean by copy(!) to each function. On the other hand I know that it is controversially discussed whether global variables should be used at all... Find some arguments here and here.
src/iGenVar.cpp
Outdated
| @@ -49,6 +49,9 @@ void initialize_argument_parser(seqan3::argument_parser & parser, cmd_arguments | |||
| parser.add_option(args.threads, 't', "threads", | |||
| "Specify the number of decompression threads used for reading BAM files.", | |||
| seqan3::option_spec::standard); | |||
| parser.add_flag(args.verbose, 'v', "verbose", | |||
| "If you set this flag to true, we provide additional details about what iGenVar does. The detailed " | |||
| "output is printed in the standard output."); | |||
There was a problem hiding this comment.
Not true, you use debug_stream, which prints to stderr.
And stderr is good, because otherwise the output would be intertwined with the vcf output (which goes to stdout).
There was a problem hiding this comment.
Instead of If you set this flag to true, just If you set this flag, because the call is ./program --verbose without true as a parameter.
test/cli/iGenVar_cli_test.cpp
Outdated
| @@ -48,6 +48,10 @@ std::string const help_page_part_1 | |||
| " -t, --threads (signed 16 bit integer)\n" | |||
| " Specify the number of decompression threads used for reading BAM\n" | |||
| " files. Default: 1.\n" | |||
| " -v, --verbose\n" | |||
| " If you set this flag to true, we provide additional details about\n" | |||
| " what iGenVar does. The detailed output is printed in the standard\n" | |||
Irallia
force-pushed
the
FEATURE/add_verbose_option
branch
from
620201b
to
e8e2001
Compare
Signed-off-by: Lydia Buntrock <lydia.buntrock@fu-berlin.de>
Irallia
force-pushed
the
FEATURE/add_verbose_option
branch
from
e8e2001
to
3fd4a4d
Compare
|
I should not work too late, then I always miss something 😅 |
Resolves partly #20
We add a verbose option here, without different values, just as a flag.