paulczar / meez

rubygem to set up opinionated TDD chef cookbook
73 stars 17 forks source link

Replace Strainer with Rake alternative. Fixes #9 #10

Closed rosstimson closed 10 years ago

rosstimson commented 10 years ago

First pass at removing Strainer in favour of a Rake based alternative as recommended by @sethvargo

The Rakefile is pretty much a straight copy of the one found in the Jenkins cookbook as referenced in the issue. Manually tested and seems to work fine, just not quite as pretty as Strainer :crying_cat_face:

jeremyolliver commented 10 years ago

Actually, I just realised the existing Rakefile here doesn't run the chefspec unit tests (rspec) either.

Here's the Rakefile that I setup the other day: https://github.com/jeremyolliver/cookbook-ruby_app/blob/master/Rakefile which also has a spec rake task. I didn't figure out a nice way to pass any command line args through to rspec, so I've just hard coded the path, and other arguments. I've also got cane style check as well as rubocop though, which is probably overkill

rosstimson commented 10 years ago

I made sure Rake was already in the bundle and figured that it was unnecessary to append the Gemfile and then try and work out the version needed for various dependencies when Bundler is already doing this job for us.

Doh, I might have known I'd forget something, I'll take another look tonight after work - I really know how to treat the Mrs on Valentines Day / Friday night :laughing:

Thanks for the link to your Rakefile I'll refer to it when I revisit this PR.

rosstimson commented 10 years ago

Should be sorted now. Done a very quick manual test, would appreciate it others have a little test before it is merged though.

P.S. Still not appending Rake to Gemfile until I hear further thoughts on this.

jeremyolliver commented 10 years ago

Ah yes, I see what you mean - rake is currently an implicit dependency included by foodcritic. It's interesting that the other gems which provide rake integration (like rpsec, rubocop here) don't have it as an explicit dependency, just optional integration. I do feel that since the project will now be using rake directly that an explicit dependency is warranted - even if there's no version specified in it - but that's probably enough of my 2 cents, I'll let someone else chip in.

rosstimson commented 10 years ago

You do have a point re the project using Rake directly. I think @paulczar should probably make this decision. I certainly can't see any real reason not to add Rake into the Gemfile though.

flaccid commented 10 years ago

+1. Rake will certainly be a dep, lets replace Strainer and move forward!

sethvargo commented 10 years ago

Rake needs to be in the Gemfile. Otherwise, you can run tests on Travis. Just don't add a version lock to rake and you should be fine.

rosstimson commented 10 years ago

Hopefully ready to pull now. I'm appending Rake into the Gemfile with no version constraint.

sethvargo commented 10 years ago

Wanna add Stove while you're in there lol?

rosstimson commented 10 years ago

Done, thanks for the advice @sethvargo. Hope I'm not treading on @paulczar's toes by changing some of this stuff that isn't strictly relevant to the original intent of this PR.

Actually I do want to add Stove, see issue #9. I've just used it on a small cookbook I released earlier today and after a few niggles I switched to using the 2.0.0 beta straight from GH. I think I might wait until 2.0.0 is released before adding it to meez, if others agree/want it added of course.

sethvargo commented 10 years ago

Oh yea. I should probably release 2.0.0. I'll do that tonightish

paulczar commented 10 years ago

I've merged this in up to including in the Gemfile ( i did that myself with some other changes I made ). it looks like you might have made a change or two while I was merging, so check if I missed anything you care about ... maybe make a new PR for them.

I also added in a rake task to run knife cookbook test, not happy with copying files to a sandbox first, but doesn't seem to be a good alternative to that.

sethvargo commented 10 years ago

@paulczar knife cookbook test is a bit pointless if you're using ChefSpec

rosstimson commented 10 years ago

My changes that weren't merged were just minor changes as per @sethvargo's recommendations:

You should be safe to just pull these in direct, I can create a separate branch and PR if necessary though.

P.S. If knife cookbook test is pointless as using ChefSpec and you are not very happy with the implementation I'd prefer to see it removed again. Just my 2 cents :smile:

P.P.S. Thanks for meez, I've already used it, coupled with Stove, to develop and release a small cookbook to the community site earlier today.

paulczar commented 10 years ago

Does chefspec cover everything that knife cookbook test does? I'm sure I've had instances where chefspec passes but knife cookbook test fails. I could be misremembering though. I should probably just re-remove it.

sethvargo commented 10 years ago

@paulczar yes, with one exception.

knife cookbook test will check if all templates are present. ChefSpec might check depending on the matcher you use. If you use create_template, it won't ensure the template exists. If you use render_file, it will ensure the template is rendered and doesn't have syntax errors.