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

Minor puppet-lint related fixes #323

Closed
wants to merge 1 commit into from
Closed

Minor puppet-lint related fixes #323

wants to merge 1 commit into from

Conversation

cryptk
Copy link

@cryptk cryptk commented Aug 22, 2016

In my use of this module, my CI system constantly complains about the two lines edited in this commit because they don't pass puppet-lint. They are very minor, almost stylistic changes that I have made to the module internally (to appease the CI system) but it would be nice to not need to run a modified version of the module.

@@ -18,7 +18,7 @@
$python = $::python::version ? {
'system' => 'python',
'pypy' => 'pypy',
default => "${python::version}",
Copy link
Member

Choose a reason for hiding this comment

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

I think this is here to ensure that python::version is a string, by casting it as a string. Not sure if it is actually needed though.

Copy link
Author

Choose a reason for hiding this comment

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

Either way, that variable will still be an int or a str based on it's contents. Quoting a variable will not cast it to a string.

root@testbox:~# cat test.pp 
$version=123
warning(type($version))
warning(type("${version}"))
root@testbox:~# puppet apply test.pp 
Warning: Scope(Class[main]): integer
Warning: Scope(Class[main]): integer

@ghoneycutt
Copy link
Member

TravisCI is broken due to ruby gem issues. Created PR #324 to fix. Once it is merged, you will need to rebase against it. This will bring in those changes and hopefully allow your tests to pass.

@cryptk
Copy link
Author

cryptk commented Aug 22, 2016

No problems, I can rebase it after #324 is merged.

@ghoneycutt
Copy link
Member

Seems I have to fix the style along with another bug to get travis passing. Did that in my PR. Thank you for bringing attention to it!

I set puppet-lint to ignore the stringification of python::version. If you still want to solve it your way by not casting it into a string, please submit a new PR once #324 is merged. Just need to make sure it passes our tests.

Thanks again.

@ghoneycutt ghoneycutt closed this Aug 22, 2016
@cryptk
Copy link
Author

cryptk commented Aug 22, 2016

I think you missed my line comment. Quoting the variable does not cast it to a string, it is an int either way.

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.

2 participants