search

JCVenterInstitute / VIGOR4

VIGOR4
GNU General Public License v3.0
17 stars 11 forks source link

ViPR modifications on VIGOR4 #19

Closed VirusBRC closed 5 years ago

VirusBRC commented 5 years ago

Please review and merge with your master branch

jchriste-jcvi commented 5 years ago

I'll make a merge branch for this.

The one functional change that I'd like to make is making the file per gene configurable since it will create a lot of redundant files for current users.

On a nit-picky level, there are some stylistic things that I'd like to change as well, like variable names not starting with a lowercase letter, not always using braces with if statements (although I see that there are some of those in the code already), etc.

Additionally, I'm afraid we're going to have no end of merge conflicts if the code is formatted differently in your repository and this one. I can add my formatter settings to the repository, but I confess that I don't have the automatic reformatter turned on and that I sometimes format things by hand.

jchriste-jcvi commented 5 years ago

I made a quick pass at matching the changes in the pull request in the trunk/brc-changes branch. I only ran it with our flu inputs, but they produced the same output as the pull request.

I still would like to make the added output file configurable and the new files should respect the --overwrite-output commandline parameter.

VirusBRC commented 5 years ago

I made a quick pass at matching the changes in the pull request in the trunk/brc-changes branch. I only ran it with our flu inputs, but they produced the same output as the pull request.

I still would like to make the added output file configurable and the new files should respect the --overwrite-output commandline parameter.

Sounds reasonable and agree. Just let us know when this is done so we can pull the codes and test in ViPR.