-
Notifications
You must be signed in to change notification settings - Fork 213
Added support of passing options for berkshelf via knife.rb #369
Added support of passing options for berkshelf via knife.rb #369
Conversation
Thanks! I think the idea itself is fine. But this breaks compatibility with Berkshelf 2 which is still widely used. Tests and documentation would also be nice. =) Travis tests seem to be broken, but that feels to be related to a new net-ssh version. |
556c949
to
05f5829
Compare
According to d8227f7 berkshelf 3+ is required
I'll update wiki as soon as this PR would be accepted. Plan to add
in the end of https://github.com/matschaffer/knife-solo/wiki/Berkshelf-&-Librarian-Chef-integration#berkshelf-integration |
9fa1315
to
ac7515c
Compare
@@ -19,7 +19,12 @@ def install! | |||
path = berkshelf_path | |||
ui.msg "Installing Berkshelf cookbooks to '#{path}'..." | |||
|
|||
berksfile = ::Berkshelf::Berksfile.from_file('Berksfile') | |||
if defined?(::Berkshelf) && Gem::Version.new(::Berkshelf::VERSION) >= Gem::Version.new("3.0.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't realize you'd added a compatibility check. Thanks!
What's the reason for the defined?(::Berkshelf)
check here? Seems like if it failed the original code as well as line 26 here will fail as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, you are right, there is no reason. I removed it.
ac7515c
to
5f1049e
Compare
Added support of passing options for berkshelf via knife.rb
Thanks! |
No description provided.