Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NO-REF Check chef client version before calling sensitive #317

Merged
merged 1 commit into from
Feb 2, 2016

Conversation

xiangyao1989
Copy link

This PR is aimed to fix the issue which chef-client with a version less than 11.14 complaining about method sensitive missing. My current project requires a chef-client version 11.12 for some reasons, and we can't depend on this rabbitmq cookbook because of the version gap in chef-client.
This fix shouldn't affect any other user but make this cookbook fit into older versions of chef client. We've tested it under chef 11.12 and it works perfect!

Refer to: chef/chef#1744 for more details.
Thanks!

@jjasghar
Copy link
Contributor

I gotta admit this is pretty simple and clever. I'll get this set up for the next release.

@jemc
Copy link
Contributor

jemc commented Oct 13, 2015

Won't comparing as a float cause problems with the minor part when a different number of minor digits is used on each side?

For example, in version numbering, 1.10 should be greater than 1.2, but in float comparisons it it less.

@xiangyao1989
Copy link
Author

@jemc Good point!
I think this one will work:

Gem::Version.new('1.10') > Gem::Version.new('1.2')
 => true 

@xiangyao1989
Copy link
Author

@jemc I just updated it to be:

sensitive true if Gem::Version.new(Chef::VERSION.to_s) >= Gem::Version.new('11.14.2')

And I made the version more accurate to 11.14.2, which is the version sensitive method introduced.
Thanks for the comment!

@xiangyao1989
Copy link
Author

@jjasghar Thanks for your quick response! We're looking forward to this fix~ : >

@jjasghar
Copy link
Contributor

Can you add a test against this also?

@xiangyao1989
Copy link
Author

@jjasghar Yep~ Just added one unit test case and verified it works~ Thanks~

@jjasghar
Copy link
Contributor

🤘 awesome thanks @XiangYao, all we need is another +1 from someone else and then we're good to release this.

@michaelklishin
Copy link
Member

This looks reasonable.

I'm not familiar enough with Chef client internals or change history to +1, though.

jjasghar pushed a commit that referenced this pull request Feb 2, 2016
NO-REF Check chef client version before calling sensitive
@jjasghar jjasghar merged commit fa940a1 into rabbitmq:master Feb 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants