Skip to content
This repository was archived by the owner on Nov 9, 2023. It is now read-only.

New open ref workflow - do not merge#1951

Closed
josenavas wants to merge 4 commits into
biocore:masterfrom
josenavas:new-open-ref-workflow
Closed

New open ref workflow - do not merge#1951
josenavas wants to merge 4 commits into
biocore:masterfrom
josenavas:new-open-ref-workflow

Conversation

@josenavas
Copy link
Copy Markdown
Member

@gregcaporaso this is the PR with the new OR workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be clearer if you did this as while len(seq_generators) > 0, rather than creating a separate done variable. Would that work? It'd let you drop the if/else, which would be nice.

@ghost
Copy link
Copy Markdown

ghost commented Mar 11, 2015

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/1599/

@gregcaporaso
Copy link
Copy Markdown
Contributor

Just a couple of comments @josenavas, looks good!

I guess we need to figure out what to do with this code... We need it for EMP, but it will never end up in a QIIME release in this form (since it would be re-written for QIIME 2). I think we should add it here, and that way you can cite QIIME 1.9.0-dev with a specific commit hash, and if we want to explore this approach further it will be accessible.

@gregcaporaso
Copy link
Copy Markdown
Contributor

Failures all look related to R packages so we'll see what happens when/if you push changes based on my comments.

@josenavas
Copy link
Copy Markdown
Member Author

Thanks for the review @gregcaporaso
I agree, merging here so we can point to it is the best route...

@ghost
Copy link
Copy Markdown

ghost commented Mar 12, 2015

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/1600/

@ghost
Copy link
Copy Markdown

ghost commented Mar 12, 2015

Build results will soon be (or already are) available at: http://ci.qiime.org/job/qiime-github-pr/1601/

@gregcaporaso
Copy link
Copy Markdown
Contributor

closing in favor of #1958.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants