search

voxpupuli / puppet-firewalld

Puppet module for managing firewalld
Apache License 2.0
40 stars 77 forks source link

fixed puppet-lint warning regarding missing namespace #291

Closed domfi closed 4 years ago

domfi commented 4 years ago

Pull Request (PR) description

Added Top-Scope namespace to $facts[] usage.

This Pull Request (PR) fixes the following issues

puppet-lint shows warnings "top-scope variable being used without an explicit namespace". This PR adds the top-scope namespace like so "$::facts[]" as mentioned in http://puppet-lint.com/checks/variable_scope/

ghoneycutt commented 4 years ago

Thanks for the pull request!!

This used to be the case, but no longer is with modern puppet versions. From https://voxpupuli.org/docs/reviewing_pr/ 

Are facts used? They should only be accessed via $facts[] or fact() function from stdlib, but not topscope variables
domfi commented 4 years ago

Sorry, just for clarification: i think $facts[] is a topscope variable. And puppet-lint warns because there is no scope given. So $::facts[] should be used I think..?

domfi commented 4 years ago

Ok, I just found this in the Puppet documentation. "The variable name $facts is reserved, so local scopes cannot re-use it." So it is ok to just use "$facts" and I have to configure puppet-lint to ignore it. Thank you for reviewing my pull request!