align puppet-lint config with coding style
Closed, ResolvedPublic

Description

Align the puppet-lint config with https://wikitech.wikimedia.org/wiki/Puppet_coding#Coding_Style and make a jenkins job vote -1 on violation.

The following checks are currently disabled in the config despite the coding style requiring them:

# WONTFIX - We want at least 100.
# Lot of long lines (ssh keys for example).
--no-80chars-check

# T127797 - needs at least one comment line for each class
--no-documentation-check

Related Objects

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 332097 merged by ArielGlenn:
dataset module: Linting changes

https://gerrit.wikimedia.org/r/332097

Change 332094 merged by Alexandros Kosiaris:
bacula module: Trailing commas, full class names

https://gerrit.wikimedia.org/r/332094

Change 332098 merged by Dzahn:
diamond module: Add trailing commas

https://gerrit.wikimedia.org/r/332098

Change 332096 merged by Dzahn:
contint module: Linting changes

https://gerrit.wikimedia.org/r/332096

Change 332099 merged by Elukey:
druid module: Linting changes

https://gerrit.wikimedia.org/r/332099

Change 332100 merged by Dzahn:
ganglia module: Use full names for class names

https://gerrit.wikimedia.org/r/332100

scfc added a subscriber: Juniorsys.Jan 23 2017, 5:18 PM

It would be nice if we had a puppet-lint check for the full names and trailing commas that @Juniorsys just fixed.

Dzahn added a comment.Jan 23 2017, 7:27 PM

@scfc +1 It should be a bug/feature request with upstream puppet-lint. Care to make one? Also, we should check if it's not just about the puppet-lint version. Might be detected in newer versions than the one we have installed in prod / on our local machines.

Change 334276 had a related patch set uploaded (by Juniorsys):
Linting fixes (Multiple modules)

https://gerrit.wikimedia.org/r/334276

Change 334278 had a related patch set uploaded (by Juniorsys):
deployment: Linting fixes

https://gerrit.wikimedia.org/r/334278

Change 334279 had a related patch set uploaded (by Juniorsys):
dnsrecursor: Linting fixes

https://gerrit.wikimedia.org/r/334279

Change 334280 had a related patch set uploaded (by Juniorsys):
docker: Linting fixes

https://gerrit.wikimedia.org/r/334280

Change 334282 had a related patch set uploaded (by Juniorsys):
etcd: Linting fixes

https://gerrit.wikimedia.org/r/334282

Change 334283 had a related patch set uploaded (by Juniorsys):
eventlogging/eventstreams: Linting fixes

https://gerrit.wikimedia.org/r/334283

Change 334285 had a related patch set uploaded (by Juniorsys):
gerrit/git/graphite: Linting fixes

https://gerrit.wikimedia.org/r/334285

Change 334286 had a related patch set uploaded (by Juniorsys):
icinga: Linting fixes

https://gerrit.wikimedia.org/r/334286

Change 334287 had a related patch set uploaded (by Juniorsys):
jupterhub/keyholder: Linting fixes

https://gerrit.wikimedia.org/r/334287

Change 334288 had a related patch set uploaded (by Juniorsys):
jenkins: Linting fixes

https://gerrit.wikimedia.org/r/334288

Change 334290 had a related patch set uploaded (by Juniorsys):
labs modules linting changes

https://gerrit.wikimedia.org/r/334290

Change 334291 had a related patch set uploaded (by Juniorsys):
ldap: Linting fixes

https://gerrit.wikimedia.org/r/334291

Change 334293 had a related patch set uploaded (by Juniorsys):
librenms/locales/logstash/lshell linting changes

https://gerrit.wikimedia.org/r/334293

Change 334294 had a related patch set uploaded (by Juniorsys):
lvm/lvs: Linting changes

https://gerrit.wikimedia.org/r/334294

Change 334295 had a related patch set uploaded (by Juniorsys):
Linting changes (multiple)

https://gerrit.wikimedia.org/r/334295

Change 334297 had a related patch set uploaded (by Juniorsys):
monitoring: linting changes

https://gerrit.wikimedia.org/r/334297

Change 334299 had a related patch set uploaded (by Juniorsys):
Linting changes (multiple)

https://gerrit.wikimedia.org/r/334299

Change 334300 had a related patch set uploaded (by Juniorsys):
ores/otrs/package_builder: Linting changes

https://gerrit.wikimedia.org/r/334300

Change 334301 had a related patch set uploaded (by Juniorsys):
openstack: Linting changes

https://gerrit.wikimedia.org/r/334301

Change 334302 had a related patch set uploaded (by Juniorsys):
phabricator: Linting changes

https://gerrit.wikimedia.org/r/334302

Change 334303 had a related patch set uploaded (by Juniorsys):
profile linting changes

https://gerrit.wikimedia.org/r/334303

Change 334306 had a related patch set uploaded (by Juniorsys):
prometheus: Linting changes

https://gerrit.wikimedia.org/r/334306

Change 334307 had a related patch set uploaded (by Juniorsys):
puppet/puppet_compiler: Linting changes

https://gerrit.wikimedia.org/r/334307

Change 334308 had a related patch set uploaded (by Juniorsys):
planet/pmacct/programdashboard/pybal lint changes

https://gerrit.wikimedia.org/r/334308

Change 334309 had a related patch set uploaded (by Juniorsys):
quarry: Linting changes

https://gerrit.wikimedia.org/r/334309

Change 334310 had a related patch set uploaded (by Juniorsys):
role: Linting changes (backup,bastionhost others)

https://gerrit.wikimedia.org/r/334310

Change 334311 had a related patch set uploaded (by Juniorsys):
redis: Linting changes

https://gerrit.wikimedia.org/r/334311

Change 334313 had a related patch set uploaded (by Juniorsys):
Linting changes (multiple)

https://gerrit.wikimedia.org/r/334313

Change 334317 had a related patch set uploaded (by Juniorsys):
Linting fixes (multiple modules)

https://gerrit.wikimedia.org/r/334317

Change 334319 had a related patch set uploaded (by Juniorsys):
graphoid/gridengine/grub/haproxy/hhvm lint fixes

https://gerrit.wikimedia.org/r/334319

Change 334320 had a related patch set uploaded (by Juniorsys):
ifttt/imagemagick/initramfs/interface lint fixes

https://gerrit.wikimedia.org/r/334320

Change 334313 merged by Alexandros Kosiaris:
Linting changes (multiple)

https://gerrit.wikimedia.org/r/334313

Change 334302 merged by Dzahn:
phabricator: Linting changes

https://gerrit.wikimedia.org/r/334302

Change 334286 merged by Dzahn:
icinga: Linting fixes

https://gerrit.wikimedia.org/r/334286

Change 334285 merged by Dzahn:
gerrit/git/graphite: Linting fixes

https://gerrit.wikimedia.org/r/334285

Change 334288 merged by Dzahn:
jenkins: Linting fixes

https://gerrit.wikimedia.org/r/334288

Change 334297 merged by Dzahn:
monitoring: linting changes

https://gerrit.wikimedia.org/r/334297

Change 334280 merged by Dzahn:
docker: Linting fixes

https://gerrit.wikimedia.org/r/334280

Change 334295 abandoned by Juniorsys:
Linting changes (multiple)

https://gerrit.wikimedia.org/r/334295

Change 334306 merged by Filippo Giunchedi:
prometheus: Linting changes

https://gerrit.wikimedia.org/r/334306

Change 334308 merged by Dzahn:
planet/pmacct/programdashboard/pybal lint changes

https://gerrit.wikimedia.org/r/334308

Change 334287 merged by Dzahn:
jupyterhub/keyholder: Linting fixes

https://gerrit.wikimedia.org/r/334287

Change 334279 merged by Dzahn:
dnsrecursor: Linting fixes

https://gerrit.wikimedia.org/r/334279

Change 334278 merged by Dzahn:
deployment: Linting fixes

https://gerrit.wikimedia.org/r/334278

Change 334300 merged by Dzahn:
ores/otrs/package_builder: Linting changes

https://gerrit.wikimedia.org/r/334300

Change 334291 merged by Dzahn:
ldap: Linting fixes

https://gerrit.wikimedia.org/r/334291

Change 334311 merged by Dzahn:
redis: Linting changes

https://gerrit.wikimedia.org/r/334311

Change 334293 merged by Dzahn:
librenms/locales/logstash/lshell linting changes

https://gerrit.wikimedia.org/r/334293

Change 334319 merged by Dzahn:
graphoid/gridengine/grub/haproxy/hhvm lint fixes

https://gerrit.wikimedia.org/r/334319

Change 334290 merged by Andrew Bogott:
labs modules linting changes

https://gerrit.wikimedia.org/r/334290

Change 334307 merged by Dzahn:
puppet/puppet_compiler: Linting changes

https://gerrit.wikimedia.org/r/334307

Change 334276 merged by Andrew Bogott:
Linting fixes (Multiple modules)

https://gerrit.wikimedia.org/r/334276

Change 334282 merged by Dzahn:
etcd: Linting fixes

https://gerrit.wikimedia.org/r/334282

Change 334283 merged by Dzahn:
eventlogging/eventstreams: Linting fixes

https://gerrit.wikimedia.org/r/334283

Change 334310 abandoned by Juniorsys:
role: Linting changes (backup,bastionhost others)

https://gerrit.wikimedia.org/r/334310

Change 334320 abandoned by Juniorsys:
ifttt/imagemagick/initramfs/interface lint fixes

https://gerrit.wikimedia.org/r/334320

Change 336668 had a related patch set uploaded (by Dzahn):
bastionhost,bugzilla: lint fixes

https://gerrit.wikimedia.org/r/336668

Change 336668 merged by Dzahn:
bastionhost,bugzilla: lint fixes

https://gerrit.wikimedia.org/r/336668

Change 336672 had a related patch set uploaded (by Dzahn):
multiple roles: lint-fix role::backup::host includes

https://gerrit.wikimedia.org/r/336672

Change 336672 merged by Dzahn:
multiple roles: lint-fix role::backup::host includes

https://gerrit.wikimedia.org/r/336672

Change 336715 had a related patch set uploaded (by Dzahn):
multiple roles: lint-fix base::firewall includes

https://gerrit.wikimedia.org/r/336715

Change 336715 merged by Dzahn:
multiple roles: lint-fix base::firewall includes

https://gerrit.wikimedia.org/r/336715

Change 336720 had a related patch set uploaded (by Dzahn):
multiple roles: lint-fix standard/base::firewall includes

https://gerrit.wikimedia.org/r/336720

Change 336720 merged by Dzahn:
multiple roles: lint-fix standard/base::firewall includes

https://gerrit.wikimedia.org/r/336720

Change 336728 had a related patch set uploaded (by Dzahn):
mirrors: lint-ignore 'passwords::mirrors not in autoload module layout'

https://gerrit.wikimedia.org/r/336728

Change 336728 merged by Dzahn:
mirrors: lint-ignore 'passwords::mirrors not in autoload module layout'

https://gerrit.wikimedia.org/r/336728

Change 336731 had a related patch set uploaded (by Dzahn):
mariadb: ingore lint warnings about autoloader layout

https://gerrit.wikimedia.org/r/336731

Change 336731 merged by Dzahn:
mariadb: ignore lint warnings about autoloader layout

https://gerrit.wikimedia.org/r/336731

Change 336735 had a related patch set uploaded (by Dzahn):
puppet-lint: remove exception for wrong autoloader layout

https://gerrit.wikimedia.org/r/336735

Change 336735 merged by Dzahn:
puppet-lint: remove exception for wrong autoloader layout

https://gerrit.wikimedia.org/r/336735

Dzahn updated the task description. (Show Details)Feb 8 2017, 11:53 PM
hashar added a comment.Feb 9 2017, 9:31 AM

--no-autoloader_layout-check is gone which is quite an achievement!

The last ones are:

  • lines to have 80 characters, which I guess is way to annoying since the Puppet does not have a good/elegant way to split long lines. I think we will keep ignoring it.
  • enforcing documentation of all classes. Which surely will take ages to complete if at all. That is tracked by sub task T127797.

With the exception of those two checks, all others have been fixed and are enforced by CI. There are two sub tasks which do not trigger puppet-lint errors:

So maybe they should no more be blockers and we can just close this parent task T93645?

Dzahn closed this task as Resolved.Feb 10 2017, 1:57 AM
Dzahn added a subscriber: hashar.

@hashar Thank you for this. Last night i was thinking about this and i was planning to write almost the same summary that you wrote above :) And i came to the same conclusion, thinking that maybe we can close this now. This is by its nature a very open-ended task, but if any time was good to call it resolved then it seems now because, yes, we do not have any more exceptions in .puppet-lint.rc besides the ones we will not fix ever or at least not ever make mandatory. I think even if we resolved the "document all classes" ticket we would not want to let jenkins vote -2 just for that. So yes, let's call it resolved. It's been a long way to here and as you say , quite an achievement. YAY! :)

Dzahn added a comment.Feb 10 2017, 2:02 AM

What can still be done here optionally is remove "lint::ignore" lines and fix the issues that they are ignoring.

manifests/site.pp:    # lint:ignore:case_without_default
manifests/site.pp:    # lint:ignore:case_without_default
manifests/site.pp:    # lint:ignore:case_without_default
manifests/realm.pp:# lint:ignore:only_variable_string
modules/statistics/manifests/user.pp:    # lint:ignore:arrow_alignment
modules/locales/manifests/all.pp:    # lint:ignore:case_without_default
modules/eventlogging/manifests/service/service.pp:        # lint:ignore:variable_scope
modules/eventlogging/manifests/service/service.pp:        # lint:ignore:variable_scope
modules/ores/manifests/web.pp:    # lint:ignore:arrow_alignment
modules/install_server/manifests/tftp_server.pp:            # lint:ignore:puppet_url_without_modules
modules/diamond/manifests/collector/extendedexim.pp:    # lint:ignore:quoted_booleans
modules/diamond/manifests/collector/localcrontab.pp:    # lint:ignore:quoted_booleans
modules/diamond/manifests/init.pp:            # lint:ignore:quoted_booleans
modules/puppetmaster/tests/puppetmaster.pp:# lint:ignore:autoloader_layout
modules/standard/manifests/diamond.pp:            # lint:ignore:quoted_booleans
modules/backup/manifests/weeklyschedule.pp:        # lint:ignore:arrow_alignment
modules/backup/manifests/schedule.pp:        # lint:ignore:arrow_alignment
modules/geoip/manifests/data/puppet.pp:  # lint:ignore:puppet_url_without_modules
modules/monitoring/manifests/host.pp:    # lint:ignore:variable_scope
modules/monitoring/manifests/service.pp:    # lint:ignore:variable_scope
modules/apertium/manifests/init.pp:    # lint:ignore:arrow_alignment
modules/logstash/manifests/output/statsd.pp:        # lint:ignore:variable_scope
modules/toollabs/manifests/check.pp:# lint:ignore:autoloader_layout
modules/sslcert/manifests/certificate.pp:    # lint:ignore:puppet_url_without_modules
modules/base/manifests/standard_packages.pp:    # lint:ignore:quoted_booleans
modules/base/manifests/environment.pp:    # lint:ignore:arrow_alignment
modules/releases/manifests/reprepro.pp:        # lint:ignore:puppet_url_without_modules
modules/releases/manifests/reprepro/upload.pp:    # lint:ignore:puppet_url_without_modules
modules/cassandra/manifests/init.pp:    # lint:ignore:only_variable_string
modules/cassandra/manifests/init.pp:    # lint:ignore:only_variable_string
modules/striker/manifests/uwsgi.pp:                # lint:ignore:single_quote_string_with_variables
modules/otrs/manifests/init.pp:    # lint:ignore:arrow_alignment
modules/role/manifests/prometheus/tools.pp:                    # lint:ignore:single_quote_string_with_variables
modules/role/manifests/lists/server.pp:    # lint:ignore:quoted_booleans
modules/role/manifests/postgres/slave.pp:        # lint:ignore:variable_scope
modules/role/manifests/cache/kafka/statsv.pp:        # lint:ignore:variable_scope
modules/role/manifests/cache/kafka/webrequest.pp:        # lint:ignore:variable_scope
modules/role/manifests/cache/kafka/eventlogging.pp:        # lint:ignore:variable_scope
modules/role/manifests/cache/instances.pp:    # lint:ignore:arrow_alignment
modules/role/manifests/logstash/puppetreports.pp:    # lint:ignore:puppet_url_without_modules
modules/role/manifests/logstash/eventlogging.pp:    # lint:ignore:puppet_url_without_modules
modules/role/manifests/logstash/apifeatureusage.pp:    # lint:ignore:puppet_url_without_modules
modules/role/manifests/logstash/collector.pp:    # lint:ignore:puppet_url_without_modules
modules/role/manifests/logging/mediawiki/udp2log.pp:    # lint:ignore:puppet_url_without_modules
modules/role/manifests/osm/slave.pp:        # lint:ignore:variable_scope
modules/role/manifests/otrs/webserver.pp:    # lint:ignore:puppet_url_without_modules
modules/role/manifests/labs/openstack/nova/compute.pp:            # lint:ignore:quoted_booleans
modules/role/manifests/labs/dns.pp:            # lint:ignore:quoted_booleans
modules/role/manifests/labs/dns.pp:            # lint:ignore:quoted_booleans
modules/role/manifests/graphite/base.pp:        # lint:ignore:arrow_alignment
modules/role/manifests/graphite/base.pp:        # lint:ignore:arrow_alignment
modules/role/manifests/access_new_install.pp:        # lint:ignore:puppet_url_without_modules
modules/role/manifests/phabricator/main.pp:    # lint:ignore:arrow_alignment
modules/role/manifests/ganglia/web.pp:        # lint:ignore:variable_scope
modules/role/manifests/mariadb.pp:# lint:ignore:autoloader_layout
modules/role/manifests/mariadb.pp:    # lint:ignore:puppet_url_without_modules
modules/torrus/tests/cdn.pp:# lint:ignore:autoloader_layout
modules/labstore/manifests/drbd/resource.pp:# lint:ignore:autoloader_layout
modules/mysql/manifests/php.pp:# lint:ignore:class_inherits_from_params_class
modules/mysql/manifests/java.pp:# lint:ignore:class_inherits_from_params_class
modules/mysql/manifests/server/package.pp:        # lint:ignore:variable_scope
modules/mysql/manifests/server.pp:# lint:ignore:class_inherits_from_params_class
modules/mysql/manifests/config.pp:# lint:ignore:class_inherits_from_params_class
modules/mysql/manifests/ruby.pp:# lint:ignore:class_inherits_from_params_class
modules/mysql/manifests/python.pp:# lint:ignore:class_inherits_from_params_class
modules/mysql/manifests/init.pp:# lint:ignore:class_inherits_from_params_class
modules/mirrors/spec/fixtures/manifests/site.pp:# lint:ignore:autoloader_layout
modules/wikimetrics/manifests/web.pp:                # lint:ignore:single_quote_string_with_variables
modules/mediawiki/manifests/hhvm.pp:        # lint:ignore:arrow_alignment
modules/bacula/tests/director.pp:# lint:ignore:arrow_alignment
modules/apache/manifests/mod.pp:# lint:ignore:autoloader_layout
modules/apache/manifests/mod.pp:# lint:ignore:right_to_left_relationship lint:ignore:autoloader_layout
modules/apache/manifests/mod.pp:class apache::mod::status { # lint:ignore:autoloader_layout
modules/ganglia/manifests/views/hadoop.pp:            # lint:ignore:variable_scope
modules/deployment/manifests/salt_master.pp:        # lint:ignore:arrow_alignment
modules/deployment/manifests/salt_master.pp:        # lint:ignore:arrow_alignment
modules/contint/manifests/packages/apt.pp:        # lint:ignore:single_quote_string_with_variables
modules/contint/manifests/hhvm.pp:        # lint:ignore:ensure_first_param
modules/contint/manifests/hhvm.pp:        # lint:ignore:arrow_alignment
modules/openstack/manifests/designate/service.pp:            # lint:ignore:ensure_first_param
modules/swift/manifests/ring.pp:    # lint:ignore:puppet_url_without_modules
modules/profile/manifests/calico/builder.pp:        # lint:ignore:puppet_url_without_modules
Binary file modules/profile/manifests/.base.pp.swp matches
modules/profile/manifests/base.pp:    # lint:ignore:quoted_booleans
modules/labs_vmbuilder/manifests/init.pp:    # lint:ignore:variable_scope
modules/puppet/manifests/self/master.pp:        # lint:ignore:variable_scope

Change 334309 merged by Andrew Bogott:
quarry: Linting changes

https://gerrit.wikimedia.org/r/334309

Change 334299 abandoned by Juniorsys:
Linting changes (multiple)

https://gerrit.wikimedia.org/r/334299

Change 334301 merged by Dzahn:
[operations/puppet] openstack: Linting changes

https://gerrit.wikimedia.org/r/334301

Change 334303 merged by Dzahn:
[operations/puppet] Linting changes for docker/etcd/kubernetes profiles

https://gerrit.wikimedia.org/r/334303

Change 334294 merged by Dzahn:
[operations/puppet] lvm/lvs: Linting changes

https://gerrit.wikimedia.org/r/334294

Change 334317 merged by Dzahn:
[operations/puppet] Linting fixes (multiple modules)

https://gerrit.wikimedia.org/r/334317

hashar removed a subscriber: hashar.Mar 3 2017, 7:18 AM

Change 322907 had a related patch set uploaded (by Hashar; owner: Dzahn):
[operations/puppet] puppet-lint: ignore 'lines over 140 chars' warnings

https://gerrit.wikimedia.org/r/322907

Change 322907 merged by Dzahn:
[operations/puppet] puppet-lint: ignore 'lines over 140 chars' warnings

https://gerrit.wikimedia.org/r/322907