search

BD2KGenomics / toil-scripts

Toil workflows for common genomic pipelines
Apache License 2.0
33 stars 18 forks source link

Add toil-lib dependency (resolves BD2KGenomics/toil#922) #434

Closed jvivian closed 8 years ago

fnothaft commented 8 years ago

I like the +/- spread on this! Reviewing now.

fnothaft commented 8 years ago

Just dropped a few nits. Other than nits, LGTM!

jvivian commented 8 years ago

Thanks @fnothaft! I'll address these now.

jvivian commented 8 years ago

@fnothaft — found a blunder in toil-lib. I'll need to push 1.0.3 out before I can merge this, doing that now.

fnothaft commented 8 years ago

Blimey mate! A blunder!

fnothaft commented 8 years ago

@jvivian so now that this is merged, PRs that added to toil_scripts.tools or toil_scripts.lib should be split, right? E.g., https://github.com/BD2KGenomics/toil-scripts/pull/381 added to toil_scripts.tools.qc. Is the new workflow that we open a PR against toil-lib, get that merged, bump the toil-lib version, and then PR the rest here? Or, actually, should new functionality go into it's own repository? E.g., should #381 be split into a PR against toil-lib and a new repository?

jvivian commented 8 years ago

That's a good question. I think the former option is what I'll do in the short term until a pipeline gets broken out into its own repo. Once a pipeline has broken up with its estranged lover toil-scripts, PRs can target the new repo with changes that complement the additions to toil-lib.

PR against toil-lib > merge > bump toil-lib to new release > build release branch on jenkins to push to pypi > bump toil-lib to dev version > PR against toil-scripts | PR against pipeline-repo.

fnothaft commented 8 years ago

SGTM. I'm not planning to touch #381 for a while, so I'll hold off there.