Small bugfixes and cleanups to BQSR #38
Conversation
|
Can one of the admins verify this patch? |
|
Jey, can you rebase this on 'master' please? You'll need to drop this change into the 'adam-core' module instead of the 'adam-commands' module. |
|
Jenkins, add to whitelist. |
|
Build triggered. |
|
Build started. |
|
Build finished. |
|
All automated tests passed. |
|
Build triggered. |
|
One or more automated tests failed |
Looks like the pull request builder is having problems? |
|
Jenkins, test this please. |
|
All automated tests passed. |
| @@ -48,6 +51,7 @@ object AdamMain { | |||
| } | |||
|
|
|||
| def main(args: Array[String]) { | |||
| log.info("ADAM starting") | |||
There was a problem hiding this comment.
If the args are not correct, this will print "ADAM starting" and then printCommands correct?
I would recommend we drop this info message or move it.
There was a problem hiding this comment.
The intention of this log message is to add a delimiter to the adam.log file to mark separate runs. (The default behavior for the adam.log file is to append, so that we don't destroy any old log data.) I'm definitely open to other ways to accomplish this, but I do think it's useful to have some kind of a delimiter printed to the log file as the very first thing so that we can tell separate runs of ADAM apart.
There was a problem hiding this comment.
How about if we move it to the else branch at line 57 where printCommands() won't be called?
There was a problem hiding this comment.
Makes sense.
How about adding the args to the message too to help with debugging then, e.g
"ADAM running with args=%s".format(args.mkString(" ")) or something like that.
Moving it to the else branch makes sense. We want to output help text when there are no args given and it would be confusing to say that ADAM is starting just before dumping help text.
|
All automated tests passed. |
| // suitable for copying and pasting back into the shell. | ||
| private def argsToString(args: Array[String]): String = { | ||
| def escapeArg(s: String) = "\"" + s.replaceAll("\\\"", "\\\\\"") + "\"" | ||
| args.map(escapeArg(_)).mkString(" ") |
|
Aside from the small nits, this is ready to merge. Thanks for the much needed cleanup, Jey! |
|
All automated tests passed. |
Small bugfixes and cleanups to BQSR
|
Thanks, Jey! |
This fixes crashes when a contig is not present in the dbSNP and cleans up some of the dbSNP and BQSR code.