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

Check for use of semi-colons used to define multiple resources #547

Open
alexjfisher opened this issue Sep 14, 2016 · 9 comments
Open

Check for use of semi-colons used to define multiple resources #547

alexjfisher opened this issue Sep 14, 2016 · 9 comments

Comments

@alexjfisher
Copy link
Contributor

Semicolons must not be used to declare multiple resources within a set of curly braces.
https://docs.puppet.com/guides/style_guide.html#resource-arrangement

eg

file {
  '/tmp/file1':
    ensure => 'file',
    mode   => '0600';
  '/tmp/file2':
    ensure => 'file',
    mode   => '0644';
  '/tmp/dir':
    ensure => 'directory';
}

It'd be nice if puppet-lint could detect this.
@rnelson0
Copy link
Collaborator

Hrm, the good and bad at that link are the same. Regardless, I believe this guidance is out of date, as this is required for the new 'default' method. I will check with Puppet to see if it is correct before proceeding either way.

@alexjfisher
Copy link
Contributor Author

The good/bad at that link are for the

resources should be grouped by logical relationship to each other

bit. They don't have good/bad examples for

Semicolons must not be used to declare multiple resources within a set of curly braces.

so I added an illustration here.

@alexjfisher
Copy link
Contributor Author

You don't actually come across many people using this style, but I did in puppet-mrepo voxpupuli/puppet-mrepo@0c86b3a

@ghoneycutt
Copy link
Contributor

This spawned from me coding like the example @alexjfisher gave and no one liking it.

Good:

file { '/tmp/file1':
  ensure => 'file',
  mode   => '0600',
}

file { '/tmp/file2':
  ensure => 'file',
  mode   => '0644',
}

file { '/tmp/dir':
  ensure => 'directory',
}

Bad

file {
  '/tmp/file1':
    ensure => 'file',
    mode   => '0600';
  '/tmp/file2':
    ensure => 'file',
    mode   => '0644';
  '/tmp/dir':
    ensure => 'directory';
}

@ghoneycutt
Copy link
Contributor

@rnelson0 what's the new default method you are speaking about? Could you please post a link?

@rnelson0
Copy link
Collaborator

@ghoneycutt
Copy link
Contributor

Given "Semicolons must not be used to declare multiple resources within a set of curly braces." in the current style guide, we should have a rule that enforces it.

@rnelson0
Copy link
Collaborator

I opened https://tickets.puppetlabs.com/browse/DOCUMENT-584 to track this concern.

@hlindberg
Copy link
Collaborator

I commented on DOCUMENT-584. The gist of that post is that the rule should be "semicolon separated multiple resource bodies should only be used in conjunction with a local default body"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants