#6 fix caret dependency ranges (RHBZ#1573064)
Closed by tomh. Opened by dcallagh.
dcallagh/nodejs-packaging bz1573064  into  master

Download 6.patch
no initial comment

I think this is right, but this logic would really benefit from some thorough unit tests so that I could be confident that this doesn't break some other case...

Actually now that I look at this, I think it fixes ^1.2 but will break ^1.2.3 (that is, caret dependencies with three parts instead of two).

Seems to work fine for me with three components - you haven't actually changed the code path for those because they match the first branch of the or condition.

Pull-Request has been closed by tomh

The more I stared at it, the more it looked wrong.

So I wrote some unit tests to convince myself I was understanding it properly. I am. This PR was wrong. :-)

Specifically it breaks the ^1.2 case:

>       assert process_dep('npm(a)', '^0.1')    == '(npm(a) >= 0.1 with npm(a) < 0.2)'
E       AssertionError: assert '(npm(a) >= 0...h npm(a) < 1)' == '(npm(a) >= 0....npm(a) < 0.2)'
E         - (npm(a) >= 0.1 with npm(a) < 1)
E         ?                              ^
E         + (npm(a) >= 0.1 with npm(a) < 0.2)
E         ?                              ^^^

Moral of the story, tests are good. Further PRs to come.

I meant, the ^0.1 case not the ^1.2 case... that function is pretty hard to follow. Anyway, followup PR is: PR#7

Metadata