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

correcting all syntax issue #238

Merged
merged 3 commits into from May 18, 2020
Merged

correcting all syntax issue #238

merged 3 commits into from May 18, 2020

Conversation

sam1el
Copy link
Collaborator

@sam1el sam1el commented May 13, 2020

Signed-off-by: Jeff Brimager jbrimager@chef.io

Making sure all cookstyle errors are cleared

Description

cookstyle errors have exposed build issues. making required corrections

Issues Resolved

Builds should complete.

Check List

Signed-off-by: Jeff Brimager <jbrimager@chef.io>
@@ -26,8 +26,6 @@ def hab(*command)

if Gem::Requirement.new('>= 14.3.20').satisfied_by?(Gem::Version.new(Chef::VERSION))
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're taking out the else here you should just take this whole thing out and require Chef 14.3. If you want to support 14.3 then put a cookstyle comment on that line to avoid the alert

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay, I wasn't sure, currently the cookbook supports back to client 12.20 and I didn't want to break any of it. would any of these changes effect that?

Copy link
Contributor

Choose a reason for hiding this comment

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

You'll want to turn that cop off with a comment then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok has that syntax changed from the foodcritic method?

Copy link
Contributor

@tas50 tas50 May 13, 2020

Choose a reason for hiding this comment

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

# cookstyle: disable x/y

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

great thank you!

@sam1el sam1el requested a review from tas50 May 13, 2020 18:59
Signed-off-by: Jeff Brimager <jbrimager@chef.io>
# context 'a Sysvinit platform' do
# cached(:chef_run) do
# ChefSpec::ServerRunner.new(
# step_into: ['hab_sup'],
Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably change this to RHEL 6 if you need a sys-v platform

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do, was going to verify if we had a rhel 6 first

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/chefspec/fauxhai/blob/master/PLATFORMS.md

Make sure to use the major versions only so use 6 not 6.9

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

only took me til this morning to get these fixed correctly lol will have them pushed in soon

…rhel 6 rather than debian for sysvinit

Signed-off-by: Jeff Brimager <jbrimager@chef.io>
@sam1el sam1el requested a review from tas50 May 15, 2020 16:40
@sam1el sam1el merged commit c092ec4 into master May 18, 2020
chef-expeditor bot pushed a commit that referenced this pull request May 18, 2020
Obvious fix; these changes are the result of automation not creative thinking.
@github-actions github-actions bot deleted the fix/cookstyle branch May 18, 2020 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants