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

Projects
None yet
4 participants
@XiangYao
Copy link

commented Oct 13, 2015

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

This comment has been minimized.

Copy link
Collaborator

commented Oct 13, 2015

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

@jemc

This comment has been minimized.

Copy link
Contributor

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.

@XiangYao

This comment has been minimized.

Copy link
Author

commented Oct 13, 2015

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

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

@XiangYao XiangYao force-pushed the XiangYao:NO-REF branch from bc243b9 to 342ef64 Oct 13, 2015

@XiangYao

This comment has been minimized.

Copy link
Author

commented Oct 13, 2015

@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!

@XiangYao

This comment has been minimized.

Copy link
Author

commented Oct 13, 2015

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

@jjasghar

This comment has been minimized.

Copy link
Collaborator

commented Oct 13, 2015

Can you add a test against this also?

@XiangYao XiangYao force-pushed the XiangYao:NO-REF branch from 342ef64 Oct 13, 2015

@XiangYao XiangYao force-pushed the XiangYao:NO-REF branch to 0b27f52 Oct 13, 2015

@XiangYao

This comment has been minimized.

Copy link
Author

commented Oct 13, 2015

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

@jjasghar

This comment has been minimized.

Copy link
Collaborator

commented Oct 13, 2015

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

@michaelklishin

This comment has been minimized.

Copy link
Member

commented Oct 13, 2015

This looks reasonable.

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

@jjasghar jjasghar removed the needs tests label Oct 19, 2015

jjasghar pushed a commit that referenced this pull request Feb 2, 2016

JJ Asghar
Merge pull request #317 from XiangYao/NO-REF
NO-REF Check chef client version before calling sensitive

@jjasghar jjasghar merged commit fa940a1 into rabbitmq:master Feb 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.