Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated DNAAPLER and Mykrobe #787

Merged
merged 17 commits into from
Nov 13, 2023
Merged

Updated DNAAPLER and Mykrobe #787

merged 17 commits into from
Nov 13, 2023

Conversation

evagunawan
Copy link
Contributor

@evagunawan evagunawan commented Nov 9, 2023

Pull Request (PR) checklist:

  • Include a description of what is in this pull request in this message.
  • The dockerfile successfully builds to a test target for the user creating the PR. (i.e. docker build --tag samtools:1.15test --target test docker-builds/samtools/1.15 )
  • Directory structure as name of the tool in lower case with special characters removed with a subdirectory of the version number (i.e. spades/3.12.0/Dockerfile)
    • (optional) All test files are located in same directory as the Dockerfile (i.e. shigatyper/2.0.1/test.sh)
  • Create a simple container-specific README.md in the same directory as the Dockerfile (i.e. spades/3.12.0/README.md)
    • If this README is longer than 30 lines, there is an explanation as to why more detail was needed
  • Dockerfile includes the recommended LABELS
  • Main README.md has been updated to include the tool and/or version of the dockerfile(s) in this PR
  • Program_Licenses.md contains the tool(s) used in this PR and has been updated for any missing

Updated main README.md for mykrobe and dnaapler for docker workshop. Updated both mykrobe and dnaapler to next version indicated by Dr. Young. Added README.md for both new versions.

Changed dockerfile.version to 2 as well as ARG DNAAPLER_VER to 0.2.0
Updated to ver 0.12.2
Updated to include dnaapler 0.2.0
Updated to include correct genotyphi and new mykrobe
Included updated mykrobe and genotyphi versions.
@erinyoung
Copy link
Contributor

These look great!

Is there any reason why dnaapler is version 0.2.0 instead of 0.4.0, which is the latest version?

Did you intend to have mykrobe be part of this PR or should I look for another PR?

@erinyoung
Copy link
Contributor

Closes #737 and #715

@evagunawan
Copy link
Contributor Author

Hi Dr. Young!

I upgraded to dnaapler 2.0 since that was the version defined in the request an update section.

I just assumed I needed to add both mykrobe and dnaapler in the same request since they are both from the workshop. Sorry about that!

@erinyoung
Copy link
Contributor

Generally we have one Dockerfile per PR for reviewer simplicity, but it's not going to break anything.

@erinyoung
Copy link
Contributor

I updated the version of dnaapler to 0.4.0 and resolved some merge conflicts. Once the tests complete, I think these files are good to merge into main.

Copy link
Contributor

@erinyoung erinyoung left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!!!

I'm going to

  1. merge this PR
  2. push the new mykrobe image to dockerhub and quay using the 0.12.2 tag and overwrite the 'latest' tag
  3. push the new dnaapler image to dockerhub and quay using teh 0.4.0 tag and overwrite the 'latest' tag

@erinyoung erinyoung merged commit 616a090 into StaPH-B:master Nov 13, 2023
3 checks passed
@erinyoung
Copy link
Contributor

Thank you, again, for your contribution!!!

You can check the status of the mykrobe deploy here : https://github.com/StaPH-B/docker-builds/actions/runs/6855078851
You can check the status of the dnaapler deploy here : https://github.com/StaPH-B/docker-builds/actions/runs/6855081475

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants