Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Move SNV caller analysis to last step in CI #169

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

jaclyn-taroni
Copy link
Member

@jaclyn-taroni jaclyn-taroni commented Oct 24, 2019

Purpose/implementation

The SNV caller step's run time has been shortened with #168 and #163, but it still is ~3x longer than any other step. Here I'm moving it to the last step (per a suggestion from @jashapiro yesterday) and (hopefully) making it more obvious where folks should add their analyses.

As a result, people submitting PRs to see if they need to make changes more quickly than if they added their analyses after this SNV step.

Alternative idea: add the "add your analysis here" after

      - run:
          name: List Data Directory Contents
          command: ./scripts/run_in_ci.sh ls data/testing

Issue

N/A - contributor friendliness

Docker and continuous integration

Check all those that apply or remove this section if it is not applicable.

  • The dependencies required to run the code in this pull request have been added to the project Dockerfile.
  • This analysis has been added to continuous integration.

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

Looks fine. Could move the "analysis here" up some, but we would want to be aware of any inter-analysis dependencies.

@jaclyn-taroni
Copy link
Member Author

Yea I agree that it could have some unintended consequences but is something maintainers should keep an eye on in general. I will leave as is for now. Everything but the SNV caller step is relatively quick, so it should not cause too much friction this way.

@jaclyn-taroni jaclyn-taroni merged commit ff04105 into AlexsLemonade:master Oct 24, 2019
@jaclyn-taroni jaclyn-taroni deleted the move-snv-ci branch October 24, 2019 22:00
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