Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to recent version of Foodcritic, fix related FC issues #466

Closed
wants to merge 3 commits into from

Conversation

hrak
Copy link
Contributor

@hrak hrak commented May 27, 2016

  • Upgrade Foodcritic to ~> 6.0
  • Define use_inline_resources in providers (FC057) if supported
  • Define source_url and issues_url in metadata (FC064 & FC065) if supported

@martinb3
Copy link
Contributor

Hello! use_inline_resources actually appears to break all of the test suites that start with ^override. This is because this cookbook relies on finding/collecting resources for users, installation, configuration, and setting up a service.

We probably need to simply disable this rule. See also Foodcritic/foodcritic#419.

@hrak
Copy link
Contributor Author

hrak commented May 27, 2016

Hi Martin, before filing the PR I ran all tests (rubocop/foodcritic/rspec) except the integration tests, because they blow up my laptop :)

I ran the ^override tests individually and could reproduce your issue. Since the cookbook relies on the resources defined in the individual providers and i can't really think of an easy way to fix this in a clean and backwards compatible way, i have updated the PR to revert the use_inline_resources changes, and have added a .foodcritic file to disable the FC057 rule.

@martinb3
Copy link
Contributor

Hello @hrak! No problem -- I think that's all we can really do (is disable the rule). Once it passes, I'll merge 👍

@martinb3
Copy link
Contributor

I've incorporated your commits/changes at 9eb4c1e. I didn't want to merge in the commits that broke it as part of the history. Thank you!

@martinb3 martinb3 closed this May 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants