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

Fix rubocop issues #70

Merged
merged 2 commits into from
Sep 10, 2015
Merged

Conversation

grubernaut
Copy link
Contributor

Instead of changing the method arguments passed to a function, actually
call the function properly. This keeps the code functioning in exactly
the same fashion as it was previously, yet updates style to conform to
rubocop's update of the SymbolProc cop.


Setting the platform configuration option will break
things. Provisioner options are generally set at the global level, and
if a developer has multiple suites of differing platforms, this will
cause things to break. By inheriting the Configurable module from
test-kitchen we are able to read the platform name for each test suite's
platform correctly. This will even read the platform correctly if
.kitchen.yml only has one test suite but multiple platforms defined.

I can't think of a solid case where the platform configuration option
would need to be set specifically different from the default.

Instead of changing the method arguments passed to a function, actually
call the function properly. This keeps the code functioning in exactly
the same fashion as it was previously, yet updates style to conform to
rubocop's update of the SymbolProc cop.
Setting the `platform` configuration option _will_ break
things. Provisioner options are generally set at the global level, and
if a developer has multiple suites of differing platforms, this will
cause things to break. By inheriting the `Configurable` module from
test-kitchen we are able to read the platform name for each test suite's
platform correctly. This will even read the platform correctly if
`.kitchen.yml` only has one test suite but multiple platforms defined.

I can't think of a solid case where the `platform` configuration option
would need to be set specifically different from the default.
@neillturner
Copy link
Owner

I think it is always good that people can override the config. There might be a case when we incorrectly determine the platform so giving option to override is good.

@ytsarev
Copy link

ytsarev commented Sep 9, 2015

@grubernaut with which version of rubocop do you hit this SymbolProc offense ? I can't reproduce with
* rubocop (0.34.1)

@grubernaut
Copy link
Contributor Author

0.34.0 included the following bug, rubocop/rubocop#2217
That was fixed in 0.34.1, which was released 10 hours ago it looks like :)

@ytsarev
Copy link

ytsarev commented Sep 9, 2015

Ok, so styling issues aside, i'm not sure if we should remove flexibility of overriding something for no real reason... I can imagine overriding puppet platform per specific kitchen platforms(sorry, unavoidable term collision) like

platforms:
  - name: platform1
    provisioner:
      platform: debian
  - name: platform2
    provisioner:
      platform: redhat

@grubernaut
Copy link
Contributor Author

Ah I can see that use case if the platform name is custom. Good point

neillturner added a commit that referenced this pull request Sep 10, 2015
@neillturner neillturner merged commit b1c6997 into neillturner:master Sep 10, 2015
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.

3 participants