genomic-medicine-sweden / gms-artic

A nextflow pipeline with a GMS touch for running the ARTIC network's fieldbioinformatics tools (https://github.com/artic-network/fieldbioinformatics).
GNU Affero General Public License v3.0
9 stars 6 forks source link

Fix GitHub workflow docker builds #80

Closed talnor closed 1 year ago

talnor commented 1 year ago

The purpose of the code changes are as follows:

Will not do

Standard test procedure

This version is a:

talnor commented 1 year ago

Looks really good. Some structural feedback:

  • Two of the self-tests are currently failing

Indeed! They have been for some time as well. I will try to fix it in another PR.

  • The building images recipes are better suited in another folder than workflow, which is currently just for the pipeline's subworkflows

I am not sure if GitHub actions will pick them up if we move them elsewhere.

  • Both the push recipes look identical. It could also be clearer in the difference of what the regular recipes and the push recipe does.

True! For each build-file, I made one file for the master branch and one to be used for testing in PRs. They differ in when they are activated and where they push the docker image to (gms-artic-nanopore vs gms-artic-nanopore-stage). Perhaps renaming the files makes it more clear?

{build_dockerfile}_on_push.yml -> {build_dockerfile}_master.yml {build_dockerfile}_on_push_stage.yml -> {build_dockerfile}_stage.yml

sylvinite commented 1 year ago

All the suggestions sounds good to me. Ill unlock the PR.