search

zoonproject / zoon

The zoon R package
Other
61 stars 13 forks source link

deprecate OneHundredBackground and OneThousandBackground #416

Open goldingn opened 6 years ago

goldingn commented 6 years ago

We really need to get rid of these duplicates of Background - Ie. just delete them from the modules repo

timcdlucas commented 6 years ago

Thinking about this, and bearing in mind our remit for reproducible workflows, we probably need something a bit more considered (certainly might for the future, if not so vital for these modules).

I think the easiest is to remove modules from GetModuleList (ideally with an argument for adding the back in i.e showDepreciatedModules = FALSE), and remove from all documentation. Then just let them sit quietly on the repo.

goldingn commented 6 years ago

There was a plan to make it possible to load modules by specific version by maintaining on the repo a list of previous module versions, and the commit at which that version was last available. That would have the same effect, since it would enable us to deprecate modules by deleting them, but still have them accessible.

The plan was to wait until we had all modules being uploaded via a website, so that that list could be automatically maintained. Though efforts on that front have stalled...

goldingn commented 6 years ago

See this on installing past versions: https://github.com/zoonproject/zoon/issues/297 and this on the lookup table: https://github.com/zoonproject/zoon/issues/291

Happy to go down other routes, though one option would be to use travis or some other webhook-type-thing to automatically update that lookup table

timcdlucas commented 6 years ago

Commit plans etc. sounds good. I guess just to be careful that if

process = OneHundredBackground worked before, I'm not sure it's ok if I need to change it to process = OneHundredBackground(commit = 1nwefej2) for it to work after depreciation. Bearing in mind that I have no idea where in the syntax the commit would be.

If the default is process = OneHundredBackground(commit = 'latest') then think that should work fine.

goldingn commented 6 years ago

Hmmm, good point. That raises the question whether we want the call to workflow to be reproducible, or the workflow itself.

Ie. I think we should ensure that doing RerunWorkflow() always works, but we probably don't need to ensure that OneThousandBackground can always be used to create a new workflow, even after it has been deprecated.