ceph / ceph-cookbook

Chef cookbooks for Ceph
Apache License 2.0
100 stars 108 forks source link

Moved _common to default. #186

Closed elliott-davis closed 9 years ago

elliott-davis commented 9 years ago

Moving _common.rb to default.rb makes this cookbook easier to include in other cookbooks. This also brings this cookbook back in line with its README.

elliott-davis commented 9 years ago

Relevant Gerrit Review

hufman commented 9 years ago

Should the include_recipe 'ceph::conf' be moved too? Most anything that talks to ceph needs to have both the software and the config file.

elliott-davis commented 9 years ago

I think you're right. Just pushed that change now.

hufman commented 9 years ago

Missed the ceph::conf in resources/cephfs.rb, but otherwise I like it!

elliott-davis commented 9 years ago

Fixed. My grepping is bad, and I should feel bad.

hufman commented 9 years ago

Running the kitchen tests reveal that each of the _install recipes includes the _common_install recipe. Should _common_install be left in place, and have the default recipe include it? I think having each of the _install recipes include the default recipe looks weird otherwise...

elliott-davis commented 9 years ago

I think they should actually be removed since default now includes it and everything should now include default.

hufman commented 9 years ago

You mean the *_install recipes should all include the default recipe? sounds good.

elliott-davis commented 9 years ago

Okay, that change has been made in the PR @hufman. Does that look good to you?

elliott-davis commented 9 years ago

ping @hufman. Anything I can do to move this patch along?

kramvan1 commented 9 years ago

Using default looks like a cleaner approach to me. Appears ready to go.

hufman commented 9 years ago

The test kitchens all passed on my end, +1 from me