Skip to content

adds cbird-util#1189

Merged
erinyoung merged 2 commits intoStaPH-B:masterfrom
Kincekara:cbird-util
Feb 5, 2025
Merged

adds cbird-util#1189
erinyoung merged 2 commits intoStaPH-B:masterfrom
Kincekara:cbird-util

Conversation

@Kincekara
Copy link
Copy Markdown
Collaborator

This is part of my C-BIRD pipeline.

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/build-files/samtools/1.15 )
  • Directory structure as name of the tool in lower case with special characters removed with a subdirectory of the version number in build-files (i.e. docker-builds/build-files/spades/3.12.0/Dockerfile)
    • (optional) All test files are located in same directory as the Dockerfile (i.e. build-files/shigatyper/2.0.1/test.sh)
  • Create a simple container-specific README.md in the same directory as the Dockerfile (i.e. docker-builds/build-files/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

@Kincekara Kincekara marked this pull request as ready for review February 4, 2025 20:04

FROM app AS test

RUN datasets -h &&\
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.

It looks like you're adding some custom scripts. Can you add some sort of test to make sure they work?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's good that at least the help options are pulled up, showing the scripts can be called and the python packages are accessible to those scripts

I don't think it's truly necessary to test out each script unless Kutluhan has an easy way to add some tests in.

I'd be OK to merge as is, considering this docker image is purpose built for C-BIRD. I imagine he will do testing of those scripts via the C-BIRD workflow prior to the next release.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @kapsakcj and @erinyoung. I already prepared a test folder and a script for that. I don't mind adding it here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, thanks for adding those 👍 Looks good to me:

#14 [test 2/2] RUN cd /test && ./test.sh
#14 0.954 test_report.docx was created!
#14 1.283 test_basic_report.html was created!
#14 1.283 test_extended_report.html was created!
#14 1.943 test_QC_summary.html was created!
#14 DONE 2.0s

@erinyoung
Copy link
Copy Markdown
Contributor

Looks good to me!

#14 [test 2/2] RUN cd /test && ./test.sh
#14 0.954 test_report.docx was created!
#14 1.283 test_basic_report.html was created!
#14 1.283 test_extended_report.html was created!
#14 1.943 test_QC_summary.html was created!
#14 DONE 2.0s

I'm going to merge this and deploy to dockerhub and quay.

@erinyoung erinyoung merged commit aa7d3c8 into StaPH-B:master Feb 5, 2025
2 checks passed
@erinyoung
Copy link
Copy Markdown
Contributor

You can check the status of the deployment here : https://github.com/StaPH-B/docker-builds/actions/runs/13161632015

@Kincekara Kincekara deleted the cbird-util branch February 5, 2025 18:39
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.

3 participants