bids-apps / freesurfer

BIDS app wrapping recon-all from FreeSurfer
Apache License 2.0
40 stars 35 forks source link

Update deployment and add version notice #77

Closed Shotgunosine closed 1 year ago

Shotgunosine commented 1 year ago

Addresses #76. docker pull bids/freesurfer, docker pull bids/freesurfer:v6, and docker pull bids/freesurfer:v6-${CIRCLETAG} will pull FreeSurfer 6.
docker pull bids/freesurfer:v7, and docker pull bids/freesurfer:v7-${CIRCLETAG} will pull FreeSurfer 7.

I've added a warning as well. If you're ok with this arrangment @Remi-Gau, I can also update the readme in this PR.

Remi-Gau commented 1 year ago

on second thought not sure that this naming won't be confusing for users

right now this would give something like:

docker pull bids/freesurfer:v6-7.4.1

Should we try something "simpler"?

docker pull bids/freesurfer:${FSversion}

with FSVersion that could be one of:

Or any version we decide to release

When we push to the default branch we release a

bids/freesurfer:${FSversion}_unstable for each FSversion

And for now latest matches "6.0.1"

And in 2024 it will match "7.4.1"

Remi-Gau commented 1 year ago

If we want to make sure we keep all the previous versions (preferable IMHO) then the release tag should be the date and we would have:

Shotgunosine commented 1 year ago

Happy to have release tag as the date. I just don't know if we want to name the Freesurfer 6 and Freesurfer 7 versions with the whole version number. That implies that we're going to continue supporting multiple Freesurfer 7 versions at the same time. How about as a compromise we have the following:

Remi-Gau commented 1 year ago

That implies that we're going to continue supporting multiple Freesurfer 7 versions at the same time

good point as long we have what FS version is in a given image I am totally OK making it clear we will NOT maintain all possible FS versions

Remi-Gau commented 1 year ago

OK good with me merging this one

Shotgunosine commented 1 year ago

Alright, if you give it a date based version tag it should deploy containers the way you wanted.

bpinsard commented 1 year ago

It seems that it didn't run the deploy step to dockerhub, I cannot see the tags. Is that expected?

Remi-Gau commented 1 year ago

yeah it is because I did not make a release, let me fix that

bpinsard commented 1 year ago

Cool thanks!

bpinsard commented 1 year ago

Looks like deploy is broken with the tags renaming.

Shotgunosine commented 1 year ago

@Remi-Gau, looks like an easy fix, just need to add: docker tag "${user_name}/${repo_name}" "${user_name}/${repo_name}:latest" before line 212 of config.yml

Do you want to just make that correction and push it directly to master?

Remi-Gau commented 1 year ago

will take care of it

bpinsard commented 1 year ago

gentle ping :D

Remi-Gau commented 1 year ago

gentle ping :D

@bpinsard

new version released are on dockerhub: https://hub.docker.com/r/bids/freesurfer/tags

bpinsard commented 1 year ago

Thanks a lot! Will deploy it asap.

SamGuay commented 12 months ago

Amazing, thank you @Remi-Gau, I was patiently waiting for this release as well!