puppet-lint questions

[Rich Burroughs]

I saw one of the Puppet Labs webinars about setting up your Puppet development environment, and at one point it mentioned using a git hook to run puppet-lint before committing. We do that where I work with "puppet parser validate," but right now a lot of our code would not pass a puppet-lint run with no options.

I'm curious how many people actually are that strict about it.

Also, I have a couple of specific questions about errors that show up in our manifests commonly.

We get this a lot:

  WARNING: quoted boolean value found on line 39

With code like this:

    $foo=hiera('foo', 'false'),

We've given a default of false for a Hiera lookup. It doesn't seem like there's a way around this, it seems like it actually needs to be quoted.

One other example:

  WARNING: double quoted string containing no variables on line 57

We get this when we are doing a tidy and are globbing:

  tidy { $log_directory:

    schedule => weekly,

    backup   => false,

    type     => ctime,

    recurse  => true,

    matches  => "foo.out*",

    age      => 97d,

  }

Again, I think this is supposed to be double quoted (I found some examples on the Puppet Labs site that had double quotes).

I know I can pass options to disable those checks, but in both cases there are times we might legitimately make a mistake that would cause that error, and I wouldn't want to ignore those...

Rich

[John Warburton]

We are in the middle of a similar task

We are manually fixing lint issues and putting a string '#PUPPET_LINT' at the end of each fixed file. The pre commit hook checks for this string before doing the lint check

All new puppet modules get the string put in immediately

At some time in the future, we will flip the logic and string change to  '#NO_PUPPET_LINT' for the challenging files and lint check by default

John

--
You received this message because you are subscribed to the Google Groups "Puppet Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/c459b945-cc6d-46c7-9dff-0ae6eab814e6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
John Warburton
Ph: 0417 299 600
Email: jwarb...@gmail.com

[Rich Burroughs]

Oh that's clever, thanks :)

Rich

To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/CAAJLFxVk3XkTi%3Dr17hJv_WBNvTX4orhJKKjDMybcTb_pGLmmHQ%40mail.gmail.com.

[Alexander Fortin]

On Friday, April 11, 2014 12:32:39 AM UTC+2, Rich Burroughs wrote:

I saw one of the Puppet Labs webinars about setting up your Puppet development environment, and at one point it mentioned using a git hook to run puppet-lint before committing. We do that where I work with "puppet parser validate," but right now a lot of our code would not pass a puppet-lint run with no options.

I'm curious how many people actually are that strict about it.

Hi Rich, in our team we use puppet-lint strictly (meaning, if a manifest can't pass any linter test, it can't be merged), and personally I like it because it help standardize the code style and in general make the manifests more readable. As far as I know, it is enforcing most (if not any) Puppetlabs style guide rules [1].

Also, I have a couple of specific questions about errors that show up in our manifests commonly.

We get this a lot:

  WARNING: quoted boolean value found on line 39

With code like this:

    $foo=hiera('foo', 'false'),

We've given a default of false for a Hiera lookup. It doesn't seem like there's a way around this, it seems like it actually needs to be quoted.

I think this is a problem with the hiera function, from a puppet point of view to me a boolean literal seems more appropriate, a string containing 'false' is semantically quite misleading. I remember I somehow solved the problem with a default '' (empty string) value for the hiera function, but maybe it's not fitting your use case as well. Another workaround is to define a default foo key in hiera itself to false. Not sure what hiera version you're running, maybe it changed with more recent versions.

 

One other example:

  WARNING: double quoted string containing no variables on line 57

We get this when we are doing a tidy and are globbing:

  tidy { $log_directory:

    schedule => weekly,

    backup   => false,

    type     => ctime,

    recurse  => true,

    matches  => "foo.out*",

    age      => 97d,

  }

Again, I think this is supposed to be double quoted (I found some examples on the Puppet Labs site that had double quotes).

I never used the tidy resource but I'm quite sure the globbing will be done at apply time and not at compile time, puppet master has no notion of what's inside the agent filesystem in the first place. Nevertheless, quoting rules are quite clear [2] and a double quote should only be needed to interpolate variables inside the string.

 

I know I can pass options to disable those checks, but in both cases there are times we might legitimately make a mistake that would cause that error, and I wouldn't want to ignore those...

Yeah, I know it's a little painful to follow them all strictly at the beginning (especially the missing-docs and >80 chars ones), but in my experience in the long run it pays off in terms of readability, so I'd advise to use puppet-lint wherever possible

[1] http://docs.puppetlabs.com/guides/style_guide.html

[2] http://docs.puppetlabs.com/guides/style_guide.html#quoting

 

-- 
http://about.me/alexanderfortin

[Daniele Sluijters]

Hi,

Our puppet-lint is fairly anal, every warning is an error in our case. But we do disable a few things:

PuppetLint.configuration.send('disable_80chars')

PuppetLint.configuration.send('disable_class_inherits_from_params_class')

PuppetLint.configuration.send('disable_class_parameter_defaults')

Sometimes things don't become more legible if you're forced to break it off somehow at 80 chars. We still try very hard to respect the 80 chars but sometimes, we go over it.

Class inherits from params class isn't really an issue. It stems from Puppet 2.6 where it didn't use to work correctly and I think from personal dislike of the author of puppet-lint for that pattern.

Class parameter defaults is the same thing, I don't find any elegance in forcing a parameter to have a default if it's a required parameter just to force me to handle that case and throw my own error message. I'm perfectly fine with the error message Puppet gives you in that case so I disable that check.

-- 

Daniele Sluijters

[Alexander Fortin]

On Monday, April 14, 2014 7:16:39 PM UTC+2, Daniele Sluijters wrote:

Sometimes things don't become more legible if you're forced to break it off somehow at 80 chars. We still try very hard to respect the 80 chars but sometimes, we go over it.

Just a note on the 80 chars limitation: in the past I found it frustrating too, but today I see it's working well with code reviews software (Gerrit) that split the window in two with the old and new version of the file.

-- 
http://about.me/alexanderfortin