Collectd version fact value

[crigertg]

fixes issue #700

[ekohl]

I think that in this case it does make sense to return false.

[bastelfreak]

Strange thing here is that our tests were green before this change, so they didn't detect this error. Conclusion: We don't have a test that proves that this change fixes anything / breaks nothing. In theory the puppetlabs_spec_helper enables STRICT_VARIABLES. It needs to be checked if this is actually the case, or if we need more tests here.

[bastelfreak]

@ctriegg cna you also please check the used email address in your commit? It isn't associated with your github account.

[ekohl]

@bastelfreak we ensure the fact is present in our tests:

puppet-collectd/spec/default_module_facts.yml

Line 2 in 15a028f

collectd_version: '5.5.0'

[alexjfisher]

I'm not sure this is the right fix. As per our own docs, we shouldn't be using top scope variables to access facts.

I think changing init.pp line 37

collectd_version_real = pick($::collectd_version, $minimum_version)

to

collectd_version_real = pick_default($facts['collectd_version'], $minimum_version)

would be better.

[alexjfisher]

Actually, I think this isn't just a style thing. This PR introduces a bug.
collectd_version_real should equal minimum_version if the fact isn't available, not false.

[alexjfisher]

This PR breaks the minimum_version parameter.

[crigertg]

alexjfisher is right.
adapting pick to pick_default is the better solution.

just running tests, pushing it when they're finished

[alexjfisher]

@ctriegg I've already got #708 open (which includes a similar fix for python_dir)

[alexjfisher]

@ctriegg Thank you for looking into this.