Various refactoring to improve code readability #149
Conversation
manifests/init.pp
Outdated
| @@ -437,6 +432,16 @@ | |||
| } | |||
| } | |||
|
|
|||
| if $::osfamily == 'Suse' { | |||
| exec { 'install /etc/init.d/snmptrapd': | |||
There was a problem hiding this comment.
Equivalent:
file { '/etc/init.d/snmptrapd':
source => '/usr/share/doc/packages/net-snmp/rc.snmptrapd',
owner => 'root',
group => 'root',
mode => '0755',
require => Package['snmpd'],
before => Service['snmptrapd'],
}Possibly with a replace => false to emulate the creates => on exec.
There was a problem hiding this comment.
PR updated. I thought about the replace => false and decided it probably isn't needed.
| path: '/etc/snmp/snmp.conf', | ||
| require: ['Package[snmp-client]', 'File[/etc/snmp]'] | ||
| ) | ||
| path: '/etc/snmp/snmp.conf' |
There was a problem hiding this comment.
I'd be happy with trailing commas. Much smaller diff :)
There was a problem hiding this comment.
rubocop won't be though :(
There was a problem hiding this comment.
We may have to teach rubocop a lesson then. There's an option for it.
There was a problem hiding this comment.
meh. Just think of the noise correcting all 100 odd modules to the new style! ;)
There was a problem hiding this comment.
thats what the rubocop:autocorrect task is for? :)
|
Cool. I'll rebase now. |
alexjfisher
changed the title
Hopefully some of the logic in (especially) `init.pp` is less complicated. It should now also be possible to split `init.pp` into the `package -> config ~> service` pattern if someone wants to take on that work. * Replace `if != and !=` with `unless == or ==` * Replace `$snmptrapd_conf_notify` with `subscribes` * Refactor `snmpv3_user` by replacing `require` with `before` * Remove duplication of identical `Service['snmptrapd']` resources * Remove some redundant relationships and reorder resources in init.pp * Refactor client.pp * Don't use absolute class names * Use proper relationship matchers in tests * Replace Exec with File resource for `/etc/init.d/snmptrapd` on Suse
|
Of course, it always helps to do a |
Refactor (and hopefully simplify) some of the conditional logic and relationships.
I've initially submitted this as a series of commits to make it easier to review (and find mistakes!).
If everything looks ok, I'll rebase and squash before merging.
In a future PR it should then be easy to move resources to separate classes and follow the
Install -> Config ~> Servicepattern.