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

Projects
None yet
2 participants
@hrak
Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

commented May 27, 2016

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Contributor

commented May 27, 2016

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

@martinb3

This comment has been minimized.

Copy link
Contributor

commented May 27, 2016

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
You can’t perform that action at this time.