galaxy-iuc / standards

Documentation for standards and best practices from the Galaxy IUC
http://galaxy-iuc-standards.readthedocs.io/en/latest/
6 stars 16 forks source link

Recommend tool `version` format #52

Closed nsoranzo closed 6 years ago

nsoranzo commented 6 years ago

Fix https://github.com/galaxy-iuc/standards/issues/51 .

peterjc commented 6 years ago

Maybe go further (integer and reset rules):

N is a wrapper-specific integer version number, starting at 0, which should be increased when you update the wrapper without changing the tool version, and reset to zero when the tool version is increased.

We could even spell out the Conda packaging build number analogy, but it might only make sense to a minority of our audience?

peterjc commented 6 years ago

I'm happy with this. Do we need a third opinion?

bgruening commented 6 years ago

I already gave my :+1: so don't feel blocked. I just wanted to point out that the +galaxy seems to be very specific to Galaxy ;) and we are trying since years to make these tools as independent as possible from Galaxy.

nsoranzo commented 6 years ago

@bgruening We can use +wrapperN is that's preferred.

martenson commented 6 years ago

I like +galaxyN. How eager do we want to be in adopting this? E.g. should https://github.com/galaxyproject/tools-iuc/pull/1798 implement this?

martenson commented 6 years ago

let's use something like <token name="@wrapper_version@“>4.3+T.galaxy1</token> per our conversation with @mvdbeek

first exception from https://github.com/galaxyproject/tools-iuc/pull/1798

mblue9 commented 6 years ago

I just wanted to point out that the +galaxy seems to be very specific to Galaxy ;) and we are trying since years to make these tools as independent as possible from Galaxy.

I thought these wrappers only worked inside Galaxy so the +galaxy made a lot of sense to me. This statement is news to me. Where else can you use these wrappers outside Galaxy?

martenson commented 6 years ago

@mblue9 You can use them anywhere you like, all of it is open source including all the parsing libraries (like https://github.com/galaxyproject/galaxy-lib/) and dependencies (Conda). We are just trying to move towards that direction and abstracting it from Galaxy is what @bgruening was pointing out imho.

mblue9 commented 6 years ago

Thanks for the clarification @martenson! I'm still feeling a bit confused but then version schemes are still dark matter to me atm so I think I'll just accept that and move on 😄

hexylena commented 6 years ago

all of it is open source including all the parsing libraries (like https://github.com/galaxyproject/galaxy-lib/) and dependencies (Conda).

and there is depends on galaxy written code to render / execute these tools. Maybe not the worst result to say "this tool description depends on galaxyproject code"?

peterjc commented 6 years ago

This looks ready to merge now.

abretaud commented 6 years ago

I think there was also the idea to have a @WRAPPER_VERSION@ token containing the +galaxyX part, I don't know if everyone is ok with this (or against it)?

bgruening commented 6 years ago

I would like to have something like @WRAPPER_VERSION@ recommended, as I think this makes auto-updates of tools easier ... maybe? One final Q from my side ... are we ok with + ending up in URL/URIs? xref: https://stackoverflow.com/questions/1856785/characters-allowed-in-a-url

nsoranzo commented 6 years ago

I would like to have something like @WRAPPER_VERSION@ recommended, as I think this makes auto-updates of tools easier ... maybe?

@WRAPPER_VERSION@ is useful only for a suite of tools, isn't it? But I think that this forces the upgrade of all wrappers at the same time, even if only one was modified.

One final Q from my side ... are we ok with + ending up in URL/URIs? xref: https://stackoverflow.com/questions/1856785/characters-allowed-in-a-url

This is encoded to %2B like slashes are encoded to %2F.

bgruening commented 6 years ago

@WRAPPER_VERSION@ is useful only for a suite of tools, isn't it? But I think that this forces the upgrade of all wrappers at the same time, even if only one was modified.

My intention was more to have this a standard variable to easily down- or upgrade it via file manipulation. So for all tools. But lets keep this out of this PR. I will experiment with it a little bit more.

This is encoded to %2B like slashes are encoded to %2F.

Q is if we want this. Or for sake of simplicity take an other letter.

nsoranzo commented 6 years ago

Q is if we want this. Or for sake of simplicity take an other letter.

The + is mandated by PEP 440 for local version identifiers, see https://github.com/galaxy-iuc/standards/issues/51#issuecomment-375500672 and following comments.

bgruening commented 6 years ago

Yes I have followed the discussion, but are we sure we want to have it, even if this means those ugly URLs? conda has gone a different route, probably for good reasons.

blankenberg commented 6 years ago

Nice discussion. I definitely think we should adopt some standard, and I think doing +galaxyN makes sense.

But I would like to point out that relying directly on a PEP in this case seems a bit counter intuitive. We're not dealing specifically with versions of python software/packages, so I don't think this PEP should constrain us. Of course it does make things convenient, since Galaxy is using Python's version parsing package.

nsoranzo commented 6 years ago

We're not dealing specifically with versions of python software/packages, so I don't think this PEP should constrain us.

Despite its origin, the PEP seems to me quite general and I don't think we want to create another standard just for nice URLs (which contain anyway / as encoded characters).

blankenberg commented 6 years ago

Totally agree with using well thought-out standards, was just pointing out the (obvious?) fact that we don't necessarily have to do it the way that Python does it.

@bgruening do you have a handle on the specific reasons and considerations that conda considered when deciding to go in another direction?

bgruening commented 6 years ago

@blankenberg no sorry. I'm not blocking this one, but I'm not 100% convinced :) Feel free to go ahead @nsoranzo.

nsoranzo commented 6 years ago

@bgruening Thanks! But I don't feel I should merge my own PR, any taker?