On Wed, Jul 23, 2008 at 1:38 AM, Xavier <shiningxc@gmail.com> wrote:
Dan McGee wrote:
This vercmp issue has been a sticking point but this should resolve many of the issues that have come up. Only a few minor code changes were necessary to get the behavior we desired, and this version appears to beat any other vercmp rendition on a few more cases added in this commit.
This commit passes all 58 vercmp tests currently out there. Other 'fixes' still fail on a few tests, namely these ones:
test: ver1: 1.5.a ver2: 1.5 ret: -1 expected: 1 ==> FAILURE test: ver1: 1.5 ver2: 1.5.a ret: 1 expected: -1 ==> FAILURE test: ver1: 1.5-1 ver2: 1.5.b ret: 1 expected: -1 ==> FAILURE test: ver1: 1.5.b ver2: 1.5-1 ret: -1 expected: 1 ==> FAILURE 4 of 58 tests failed
Well, I don't really care how you fix it. But my point was that everyone was used the old vercmp behavior, and no one reported any bugs or unexpected behavior that would justify to change this code. For example, I am not aware of anyone reporting the above cases. So I just went for the "If it's not broken, don't fix it" way.
Now if you think the new code is better and also offer a better behavior, that's fine.
No hard feelings if it came across that way. :) I was mostly just curious how different the two implementations were, so I played around a lot last night. I have one commit where I mashed the two up, and then I stumbled upon what was basically a 3 line change here to restore the old behavior. Two of the changes were mistakes in the pkgrel handling, and the other simply dealt with the assumption that leftover characters always indicated a greater version. Does anyone have any thoughts on the behavior of the above new test cases? Obviously these are not common in real life at all, but they at least seemed valid to try and place in our ordering system as I attempted above. -Dan