charm proof needs min-juju-version support #141

Closed
ryan-beisner opened this Issue Mar 21, 2016 · 8 comments

Comments

Projects
None yet
5 participants

Please add min-juju-version support to charm proof in preparation for the feature landing. This will allow us to add the metadata to development versions of charms and pass proof. TIA

Owner

marcoceppi commented Mar 21, 2016

validation should be any version number that is X.Y or X.Y.Z where X >= 2

@marcoceppi marcoceppi added this to the 2.0 milestone Mar 21, 2016

Member

tvansteenburgh commented Mar 21, 2016

Note that versions such as "2.0.1-beta3" should also be acceptable.

Owner

marcoceppi commented Mar 21, 2016

should they?

Yes, juju-min-version is parsed using our standard version parsing library, so supports stuff like build tags of beta3, which get compared lexicographically. The full code is here for parsing: https://github.com/juju/version/blob/master/version.go#L150 and here for comparison: https://github.com/juju/version/blob/master/version.go#L193

That code should be pretty trivial to convert to python, if needed.

Owner

marcoceppi commented Mar 21, 2016

bleh, okay so

Valid

  • 2.0.1
  • 2.1
  • 2.10.2-beta1
  • 2.10-beta10

Invalid

  • 1.26.0
  • 2-beta6
  • 0.6.10
  • 1.25

Does this seem correct?

@tvansteenburgh tvansteenburgh self-assigned this Mar 21, 2016

Contributor

mbruzek commented Mar 21, 2016

Reading the code link that @natefinch provided @marcoceppi it appears your 3rd case is not valid.

2.10.beta1.2 appears to be the format the code is looking for.

Correct me if I am wrong on that read.

Owner

marcoceppi commented Mar 21, 2016

It seems we just need this:

^(\d{1,9})\.(\d{1,9})(\.|-(\w+))(\d{1,9})(\.\d{1,9})?$

From version.go#L104

tvansteenburgh added a commit to tvansteenburgh/charm-tools that referenced this issue Mar 21, 2016

@marcoceppi marcoceppi closed this in #146 Mar 21, 2016

marcoceppi added a commit that referenced this issue Mar 21, 2016

Merge pull request #146 from tvansteenburgh/issues/141
Add min-juju-version support to charm-proof. Fixes #141.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment