naneau / semver

PHP Semantic Versioning library
MIT License
73 stars 9 forks source link

Equal versions compare as TRUE #10

Closed fideloper closed 10 years ago

fideloper commented 10 years ago

Hi! I have two version strings (4.0.0 and 4.0.0). When I compared them using greaterThan(), the result is true:

For example:

$versionA = Parser::parse('4.0.0');
$versionB = Parser::parse('4.0.0');

Compare::greaterThan($versionA, $versionB); //true

I think this should result in false, but let me know if I'm missing something (perhaps semver spec or if you just disagree :D )

naneau commented 10 years ago

Nope, that seems like a bug! I'll look into that.

fideloper commented 10 years ago

Thank you! Looking at the code, it's not immediately obvious where the bug is (naturally, as bugs go :D), but perhaps a short-circuit could be to compare the results of __toString to test equality:

function equals($v1, $v2) {
    if( (string)$v1 === (string)$v2) ) { return false; } // whoops, should return true
}

(I'm sorry if that's too simplified to work within semver, it very well could be missing some nuances I'm ignorant of)

naneau commented 10 years ago

If two semver strings are exactly equal, they can not be... different? :)

fideloper commented 10 years ago

Oh right, that example is absolutely terrible lol

I meant this:

function equals($v1, $v2) {
    if( (string)$v1 === (string)$v2) ) { return true; }
}

In my head, I was within the greaterThan() method, where something like:

function greaterThan($v1, $v2)
{
    if( (string)$v1 === (string)$v2) ) { return false; }
}

... might make sense to return false

naneau commented 10 years ago

I get what you mean, will look into it asap

naneau commented 10 years ago

Fixed in 54ee69549f4f977c673d582464ecb4960ce6f560

fideloper commented 10 years ago

Thank you!

On Tuesday, June 17, 2014, Maurice Fonk notifications@github.com wrote:

Closed #10 https://github.com/naneau/semver/issues/10.

— Reply to this email directly or view it on GitHub https://github.com/naneau/semver/issues/10#event-132279284.