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

False positive in "optional parameter listed before required parameter"? #126

Closed
incase opened this issue Aug 7, 2012 · 3 comments
Closed

Comments

@incase
Copy link

incase commented Aug 7, 2012

Hi.

We have this define:

define apt_package($configfiles=undef,$ensure='latest') {
  package { $name: 
    ensure      => $ensure,
    configfiles => $configfiles,
    provider    => retryapt,
    require     => Class['apt::update'],
  }
}

Running puppet-lint on it results in:

WARNING: optional parameter listed before required parameter on line 3
WARNING: optional parameter listed before required parameter on line 4
WARNING: optional parameter listed before required parameter on line 3
WARNING: optional parameter listed before required parameter on line 3
WARNING: optional parameter listed before required parameter on line 4
WARNING: optional parameter listed before required parameter on line 3
WARNING: optional parameter listed before required parameter on line 4
WARNING: optional parameter listed before required parameter on line 3
WARNING: optional parameter listed before required parameter on line 3
WARNING: optional parameter listed before required parameter on line 4

However, I can't reproduce with a stripped down file (like posted above), only with the full file. I will send you the complete file on request. It might contain non-public stuff, so I don't want to publish it completely here. I'm trying to strip it to the minimum required to reproduce and see if I can post that.

@rodjek
Copy link
Owner

rodjek commented Aug 7, 2012

Please try to narrow down where in your file it's coming from. If I can't reproduce the problem, I can't fix it.

@incase
Copy link
Author

incase commented Aug 8, 2012

The bug doesn't occure if I use the dust_bunny branch....
Here is the minimal version I was able to fully reproduce the problem, note that removing any of the empty defines removes one or two of the warnings, while removing the package resource from the apt_package resource removes them all:

define apt_package($ensure='latest',$configfiles=undef) {
  package { $name:
    ensure      => $ensure,
    configfiles => $configfiles,
    require     => Class['apt::update'],
    provider    => retryapt,
  }
}

define apt_repo($ensure='present',
                $distribution='',
                $release='',
                $repo_stage='',
                $src_repo=true,
                $uri='http://example/repo',
                $source_dir='/etc/apt/sources.list.d',
                $components='main',
                $header='Puppet Managed Apt Repo') {

}

define apt_pin($ensure='present', $explanation='', $package='', $pin='', $priority='') {
}

define gsysnews($ensure='present',
                $title='',
                $description='',
                $displayif='',
                $command='',
                $terminal='',
                $period='') {

}

define crond_entry($ensure='present', $comment='', $path='', $mailto='',
  $user='', $command='', $minute='', $hour='*', $monthday='*', $month='*', $weekday='*',
  $interval='') {

}

define k5login_entry($primary='', $instance='', $realm='KRB5.EXAMPLE.COM', $file='/root/.k5login', $ensure='present') {
}

define gconftool($key, $value='', $type='', $listtype='', $configsource='mandatory', $unset=false) {
}

It would also be of interest to me what exactly the check is supposed to detect. Wrong ordering of parameters in resources (resource instance definition), or in parameter lists in defines (when creating the define, not using it)?

@rodjek
Copy link
Owner

rodjek commented Aug 8, 2012

This check covers section 11.8 of the style guide (display order of parameters).

In parameterized class and defined resource type declarations, parameters that are required
should be listed before optional parameters (i.e. parameters with defaults).

As this has already been resolved in the development branch for the 0.2.0 release, I'm going to close this issue.

@rodjek rodjek closed this as completed Aug 8, 2012
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

2 participants