Kebabify special argument collection args.

[cmnbroad]

Fixes #120.

[codecov-io]

Codecov Report

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

@@           Coverage Diff           @@
##             master   #123   +/-   ##
=======================================
  Coverage        76%    76%           
  Complexity      591    591           
=======================================
  Files            22     22           
  Lines          2205   2205           
  Branches        456    456           
=======================================
  Hits           1676   1676           
  Misses          352    352           
  Partials        177    177

Impacted Files	Coverage Δ	Complexity Δ
.../barclay/argparser/SpecialArgumentsCollection.java	100% <ø> (ø)	1 <0> (ø)	⬇️

---

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 f2e7eac...8d12094. Read the comment docs.

[cmnbroad]

@lbergelson Can one of you take a look at this ?

[cmnbroad]

@vdauwera maybe ? I'd like to get this in for the barclay release tomorrow. Its pretty small.

[lbergelson]

This looks good to me. I suspect Magic is going to want an option to provide his own implementation that changes the names. Do we want to interfacize this preemptively?

[magicDGS]

@lbergelson is right, I would like to have a way to provide my own for my own transition to kebab-case, scheduled not soon enough to be able to apply it by Jan 9th. Thank you in advance!

[cmnbroad]

@magicDGS I recognize that ideally these would be configurable, but we won't be able to do it in time for the initial GATK release. Is your concern around downstream tools based on GATK (which is still in beta - there is no guaranty such changes won't be made - as you know we're changing all of the args to kebab case for the initial release). Or tools with a direct dependency on Barclay, independent of GATK ?

If its the latter, I recognize that this would be a breaking change for which there is no good work around without a customization facility. The only other option we have though is to not take the PR, which will mean we'll need to make a breaking change later in GATK. At some point in the future we'll implement a more extensive customization facility that enables consumes to provide these arg names, like the one you started in #95. Please let me know if you can tolerate this change in the meantime. I need to do a Barclay release early tomorrow.

[magicDGS]

Ok, I can tolerate that... although it is a breaking change, I think that no user of ny downstream software is using the arguments in this PR. Thanks for considering my concerns!

[magicDGS]

I was writing all the comments without having access to a proper computer to add a detailed comment on this. I think that it is much simpler to add the CommandLineParserConfig implementation of #95, including the special argument flags (getHelpFlag, getShowHiddenFlag, getArgumentsFileFlagp). As configurable options grow, this will be more flexible by extending the configuration file and overriding the needed methods.

If you have a look to that PR I can include the getters for the flags for having it for release of GATK4 and do not break compatibility in Barclay again when making this configurable...

[cmnbroad]

I had to cut the release in time to get into picard/gatk, so this didn't make it in.

[magicDGS]

For making it more general, we can add some getters with default values to CommandLineParserConfig (see #126)

[lbergelson]

@cmnbroad Can we merge this one? I think Magic's objections are valid but I think changing this an revisiting them later if someone expresses interest again would be a good idea.