Bug #21769

[sergeyhush]

See bug description https://projects.puppetlabs.com/issues/21769#change-94765

[puppetcla]

CLA signed by all contributors.

[adrienthebo]

This conditional seems a bit inside out to me. Would unless defined?(Float::INFINITY) be more or less readable to you?

[sergeyhush]

I hate 'unless' but that is my personal taste. I would not mind putting this in if the logic is sound and if you guys decide that it looks too ugly for your taste, replace or wait until you deprecate 1.8 support and remove that ugliness all together.

[adrienthebo]

Thank you very much for this contribution!

This seems like a great change and I'm happy to merge it in. However I want to confirm - right now this monkey patches Float::INFINITY on Ruby 1.8 using the (1.0/0.0) syntax, but that will fail on Debian PPC with Ruby 1.8, correct?

[sergeyhush]

It is easier to show than to tell - below is what I get in the debugger:

root@host:~/puppet# bundle exec rspec spec/unit/semver_spec.rb -d
Run options: exclude {:broken=>true}
........./root/puppet/spec/unit/semver_spec.rb:114
tests.each do |vstring, expected|

[109, 118] in /root/puppet/spec/unit/semver_spec.rb
109 '1.2.x' => SemVer.new('v1.2.0') ... SemVer.new('v1.3.0-'),
110 '1.x' => SemVer.new('v1.0.0') ... SemVer.new('v2.0.0-'),
111 }
112
113 debugger
=> 114 tests.each do |vstring, expected|
115 SemVer[vstring].should == expected
116 end
117 end
118
(rdb:1) SemVer::MAX.major
-Infinity
(rdb:1) 1.0/0
-Infinity
(rdb:1) 1.0/0.0
Infinity

So on 1.8 and 1.9 if we are trying to get the value of 'Infinity' 1.0/0.0 WILL work, but 1.0/0 will fail(show -Infinity instead of +Infinity) on Debian PPC with both1.8 and 1.9 .

[adrienthebo]

So 1.0/0 evaluates to -Infinity. Wow. That makes my head hurt. Thanks for the explanation!

[adrienthebo]

One last thing - the commit message could stand a minor rewording. I'll do it myself now so we can get this merged in without any delay, but for future reference this is my snippet on the topic:

It would be good to have the commit message amended a little. The first line of the commit should have the issue number and a description of the problem or fix, and the body of the commit message should explain what is this existing behavior and how this change fixes it. We have a more complete explanation of this at https://github.com/puppetlabs/puppet/blob/master/CONTRIBUTING.md#making-changes .

[adrienthebo]

Oh expletives, Github masked the commit message for me. Ignore the above comment.

[adrienthebo]

summary: merged into master in 4b06092; this should be released in 3.3.0. Thanks again for the contribution!