mattray / spiceweasel

Generates Chef knife commands from a simple JSON or YAML file.
Apache License 2.0
284 stars 65 forks source link

2.8 data bags #82

Closed andyglick closed 10 years ago

andyglick commented 10 years ago

Matt,

What you will find in this set of mods are changes that reduce the number of rubcop complaints. I didn't attempt to reduce the cyclomatic complexity of methods, but I did take a whack at what seemed to me to be low hanging fruit. I reduced the number of rubocop notifications to around 20 down from around 50.

It also occurs to me to say that I may have pushed the versions of various gems beyond where you might want them to be. You can check the Gemfile and the spiceweasel.gemspec to see what I pushed.

My goal is to keep everything in sync with chefdk, which currently is at release 0.2.2, which is what I am running. Please let me know what versions of which gems that you want spiceweasel to support.

Thanks,

Andy

I didn't include the fix for the databags issue that I opened as issue 79. When I was coding a possible solution I determined that there was code that handled data_bags in data_bags.rb and in extract_local.rb, and I wanted to see if I could move the logic to one place, rather than have it in 2 locations.

mattray commented 10 years ago

I've got a webinar tomorrow that I should be focusing on, this is a large patchset. First off though, we need to make TravisCI happy. I'll try to take a deeper look Tuesday afternoon.

andyglick commented 10 years ago

Matt -- here is what I see that looks really weird. Under travis.ci rubocop is choking on the file test/examples/example.rb. I have been running rubocop from the top level directory. When I do that rubocop only complains about the ruby files in the source tree. It is ignoring the ruby files in the test tree, and the file test/examples/example.rb is what is breaking on travis. What is weird about that is that I didn't change that file, when I display git annotate it is telling me that you and someone named Roberts are responsible for all of the code in that file. It seems weird that travis should now be failing a file that hasn't changed.

Did something cause the travis runs to start executing rubocop on the example.rb file?

In fact I did change the .rubocop.yml file and committed the changes. rubocop was itself complaining about the format of the lines in the yml file. Perhaps that is causing different behavior?

I can see the rubocop exceptions if I run

rubocop test/examples/example.rb -- I will clean that up

andyglick commented 10 years ago

OK -- I did the clean up on the example.rb file -- it isn't generating any rubocop complaints -- it should be good to go in addition, using ruby mine I did a mess of refactoring in the main body of the code. i got the number of rubocop complaints down to 13 what I didn't do was mess with the gem versions -- I figured you would have a better idea of what you want them to be

andyglick commented 10 years ago

the current issue with travis ci is another rubocop violation. In 2 places I am attempting to return multiple values from a method -- ruby mine wants the lines to look like "return x, y" or "return x, y, z" while rubocop claims that the "return" is redundant

when I execute the rspec tests without the "return" on the line all of the tests fail, when I execute rspec with the "return" only 22 of the tests fail, so that sounds as if the rubocop constraint is incorrectly being applied. I was only expecting to see 20 test failures because I was pretty sure that when I finished making the changes last night only 20 tests were failing, but I think that I can live with 2 additional failures, which I will attempt to fix if and when we get this pull request resolved :-)

I was able to locate the following rubocop configuration block and I have applied it to .rubocop.yml. Now when I run rubocop it no longer reacts to the "return w, y" lines.

Style/RedundantReturn:

When true allows code like return x, y.

AllowMultipleReturnValues: true

I'll add this latest change to the pull request payload.

andyglick commented 10 years ago

Matt,

I am confused because on my machine I see 22 rspec failures, and the travis ci server sees 2 failures and so I'm not sure how to proceed. Since I'm not able to produce an equivalent environment I'm not sure what to try next.

I'd appreciate any insights that you would be willing to share.

Thanks,

Andy

mattray commented 10 years ago

Perhaps you're not using Rubocop 0.18.1? I pinned it to that awhile back (the OpenStack cookbooks and a few other folks did too), Rubocop seemed to be changing their opinions frequently. https://github.com/mattray/spiceweasel/blob/2.8/spiceweasel.gemspec#L30

I'll try merging this myself and try to make Travis happy.

andyglick commented 10 years ago

The issue with the travis build isn't rubocop, rubocop succeeded on this build

https://travis-ci.org/mattray/spiceweasel/builds/36685599

there is a build for each ruby 1.9.3, 2.0.0 and 2.1.1

the individual builds are labeled 155.1, 155.2, 155.3

what is causing the build to fail is 2 rspec errors. What I said about my environment is that I am getting 22 rspec errors rather than 2 rspec errors. In addition while I am very familiar with JUnit I am not at all familiar with rspec and I'm not sure how it works.

rspec ./test/spec/chef_client_spec.rb:58 # --chef-client from 2.5 --chef-client, rb rspec ./test/spec/spiceweasel_spec.rb:146 # bin/spiceweasel maintains consistent output from the example config with rb

I will see if I can make any progress getting the 2 failing tests to work

andyglick commented 10 years ago

I now see that I didn't look carefully enough at the travis ci output for build 155.1 for ruby 1.9.3. That run was producing a rubocop error apparently because there was no # encoding: utf-8 comment. Have added that and pushed it.

Will continue working on the 2 rspec errors.

mattray commented 10 years ago

I cleaned this up on this on my flight, here's the branch I pushed: https://github.com/mattray/spiceweasel/tree/pr82

There's an issue in the ffi-yajl that is the cause of some of the TravisCI failures: https://github.com/opscode/ffi-yajl/issues/17

mattray commented 10 years ago

This is in the 2.8 branch, so I'm closing this one.