Skip to content

Conversation

@pbrisbin
Copy link
Contributor

@pbrisbin pbrisbin commented Mar 8, 2016

/cc @mrb @codeclimate/review

The current engine is deployed from here, which includes some
bug-fixes to the underlying tool.

This repo copied over its wrapper script with the following changes:

  • General refactor

  • Use the tool as-is

  • Pass (essentially) include_paths/**/*.rb as cookbook paths

  • Don't use the tool's exclude_paths feature

  • Support config.tags

  • Support config.cookbook_paths

    NOTE: If present, our own computed include_paths are ignored

  • Hide issues on non-existent files

Comparing on an example repo:

Existing engine: https://codeclimate.com/github/mrb/mysql/issues

This version (required excluding test and spec):

== libraries/provider_mysql_service_base.rb (1 issue) == 78: Avoid repetition of resource declarations [foodcritic] == libraries/provider_mysql_service_smf.rb (1 issue) == 6: Prefer conditional attributes [foodcritic] == libraries/provider_mysql_service_systemd.rb (1 issue) == 6: Prefer conditional attributes [foodcritic] == libraries/provider_mysql_service_upstart.rb (1 issue) == 6: Prefer conditional attributes [foodcritic] 

(The same.)

@pbrisbin
Copy link
Contributor Author

pbrisbin commented Mar 8, 2016

I should probably write some tests too.

@wfleming
Copy link

wfleming commented Mar 8, 2016

i-should-write-s0z31y

Dockerfile Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a -slim version of this we can use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look and hope it compiles -- been running into some nokogiri issues.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yeah, I think you need the ruby-dev package (& maybe some build tools as well), which -slim images don't come with. Bummer. If we don't need 2.3, maybe stick with our alpine image for now? The full Ruby image is pretty large, if I recall. (I should really make time to update our alpine image for newer Rubies).

@pbrisbin
Copy link
Contributor Author

pbrisbin commented Mar 8, 2016

OK, this is ready for final review.

@mrb once merged, how would you like me to proceed on this? I propose:

  • Get your approval (i.e. please test it)
  • Open source this repo
  • Add to submissions
  • Ship it as the new version of this engine
  • Update docs accordingly (see this repo's README)
@jpignata
Copy link
Contributor

jpignata commented Mar 9, 2016

I tested this engine across a a handful of repos from https://github.com/chef-cookbooks and it looks 👍 to me.

This is an extracted combination of mrb/foodcritic (the code_climate_support branch) and chef/codeclimate-foodcritic and is meant to obviate both of those in a codeclimate-community-maintained version. It has the following notable differences from those projects: - Invokes foodcritic as-is (does not rely on bugfixes in the mrb fork) - Expands and filters include_paths into .rb files as necessary - Supports config.tags - Supports config.cookbook_paths (means ignoring include_paths) - Hides issues on non-existent files (CC does not support this)
@pbrisbin
Copy link
Contributor Author

pbrisbin commented Mar 9, 2016

@jpignata I've rebased with (hopefully) a good commit message. If you agree, please feel free to merge, open source this repo, and begin the process of getting it in front of the customer.

I have to run, but hope to be back online ~10:15.

jpignata added a commit that referenced this pull request Mar 9, 2016
@jpignata jpignata merged commit 214aeec into master Mar 9, 2016
@jpignata jpignata deleted the pb-initial branch March 9, 2016 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants