Account for missing numeric segments while comparing Pod Versions

[gabro]

As discussed in CocoaPods/cocoapods.org#185 (comment)

Long story short, now

3.0.0-alpha.1 < 3.0-beta.9

I haven't been too creative: I simply handled major, minor and patch explicitly then just used the same comparison algorithm as Gem::Version for the remaining segments.

@floere I tried to write some tests for the "funky" versions you pointed me at (http://search.cocoapods.org/api/v1/pods.facets.json?include=version&only=version&counts=false) but apparently Version.new("20aa2b2b15b6f8db350ec07b6041e4951bb255d0") is not a valid Pod::Version (the initializer fails)

Same goes for other version strings such as "beta.9", "2-beta" and similar.

[segiddins]

alignment here is off

[gabro]

good catch, thanks

[segiddins]

This generally looks good, but please take a look at the spec failures? https://travis-ci.org/CocoaPods/Core/jobs/72843992#L1216

[floere]

@gabro Would it be possible to use a super call to avoid code duplication? Apologies if it's obviously not possible – I only am able to browse via mobile phone.

[gabro]

@floere I though about it but I didn't come up with an elegant solution.

<=> takes an other version as input parameter, and compares it with self. If self has a missing patch number, we would be back to square 1.

[floere]

Should this be Gem::Version?

[gabro]

Possibly. Do we want to allow comparison between Gem and Pod versions?

[floere]

@gabro Thanks! I see the issue – the method is too large in Gem::Version to cleanly code around it by using super.

[floere]

Good question. In a world where only Pod::Versions are used in Core, we probably don't.

I was possibly thinking of Liskov when asking this.

@segiddins @gabro In any case, with succeeding specs I am happy to pull :)

[gabro]

Ok, this is trickier than I thought and somewhat related to the Pod::Version vs Gem::Version matter.
We would want to always use Pod::Version in comparisons, but this is not always true.

The reason specs are failing is that Pod::Requirement inherits initialize from Gem::Requirement and uses the parse implementation from the superclass.
This results in versions that are Gem::Version instead of the expected Pod::Version, so the comparison fails.

I should have fixed it in my latest commit, although it's not pretty (code duplication).

[segiddins]

Looks from the failing specs like you might need to override none? as well?

[gabro]

Yes, I was looking into it right now.

[gabro]

Ok, almost there.

There were a few style issues: I fixed most of them, but two of them I chose to ignore:

• constants naming convention for DefaultRequirement (since it's an override and it's used by the super implementation)
• perceived complexity of the <=> method. I'm not super happy with that piece of code and there's clearly room for improvement, but 90% of it is already working in production, so I'm very reluctant to refactor it.

[segiddins]

thanks so much, @gabro! I'm going to make a few tweaks and add a few more tests, and then merge! Great work!

[gabro]

Thanks to you guys for the great work! I'm glad I could give a little contribution!

[segiddins]

Manually merged via 74f8fdc.