galaxyproject / ephemeris

Library for managing Galaxy plugins - tools, index data, and workflows.
https://ephemeris.readthedocs.org/
Other
27 stars 38 forks source link

Done: Refactor shedtools #104

Closed rhpvorderman closed 5 years ago

rhpvorderman commented 5 years ago

After two attempts refactoring the existing file, I am going to refactor shed tools from scratch. I will use the issue #91 as a guideline.

Please do not add any functionality to shed tools before the refactoring complete. I will be pushing regularly so you can check on my progress. I will also be available on gitter.

Done. The following stuff has changed:

The following stuff has not changed:

Stuff that needs further disccussion: + The CLI interface has some mentions of skipping tool dependencies and installing resolver dependencies. Skipping tool dependencies and installing resolver and repository dependencies are now the default, so these options do not make sense. They are ignored in def main but kept for backwards compatibility. (I have broken every one's scripts already once... )

I hope it is much easier to work now on shed_tools. Should also be a lot easier to write a testing framework now using pytest #80 .

Hope this somewhat redeems myself for not being at the galaxy hackathon.

rhpvorderman commented 5 years ago

Done!

rhpvorderman commented 5 years ago

Added extra CLI functionality to solve #101

mvdbeek commented 5 years ago

I think I have implemented all those changes in https://github.com/rhpvorderman/ephemeris/pull/3

rhpvorderman commented 5 years ago

Thanks for all the changes @mvdbeek. Especially for fixing the error of get-tool-list listing deleted tools. I ran into this error, but did not really understand why it was happening, and then forget about it. Thanks for all the effort on this pull request!

This is nice, I like it in principle. I'm ambivalent on the name change of install_repositories to install_tools, because we only install repositories, and I'm afraid that distinction is lost with these changes. Which is sort-of OK, because nothing here actually deals with tools, and we were not very consistent from the start.

Yes this is quite ambivalent trough ephemeris. get-tool-list also gives you a list of repositories. So making this consistent across ephemeris would mean we have to use get-repository-list. Internally it can be made consistent though, to avoid confusion.

Since we are going to break planemo anyway, I better do that now to avoid breaking it twice. Commits coming up.

rhpvorderman commented 5 years ago

Renamed all the tools that are actually repos to repos.

I kept the test_tools method, since it actually tests tools. It tests all the tools in a certain list of repos. So kept that the same. I hope it is not too confusing.

Also kept the user interface the same. I can rename everything in the documentation to repositories properly, and also change the information in the parser. But I do not think that is something we should do in this pull request.