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

move params to data-in-modules #67

Merged
merged 1 commit into from
Mar 12, 2018

Conversation

bastelfreak
Copy link
Member

This got pulled out of #65. The lowest recommended puppet version for this is 4.10.10. In the Vox Pupuli namespace we had a lot of argumentation if this requires a major version bump or not. From a point of defensive programming this should be a major bump.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this doesn't really add anything. We're not using params.pp and that's very clear. It's easy to follow. Data in modules IMHO only makes sense when you have a big case statement in params.pp with nested exceptions for some versions.

systemd::networkd_ensure: 'running'
systemd::manage_timesyncd: false
systemd::timesyncd_ensure: 'running'
systemd::ntp_server: ~
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the best practice is, but I think it's cleaner to have any Optional type in init.pp default to undef.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK best practice is ~, which is also used in the puppetlabs modules.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would be more consistent to have all parameters in hiera.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I now see #65 where it does make sense.

@raphink
Copy link
Member

raphink commented Mar 7, 2018

LGTM, but I think there should be a warning in the README that you need to use the Hiera 5 format to use the module from then on.

@raphink raphink merged commit 973bade into voxpupuli:master Mar 12, 2018
@bastelfreak bastelfreak deleted the datainmodules branch March 12, 2018 09:21
op-ct pushed a commit to op-ct/puppet-systemd that referenced this pull request Jun 17, 2022
Emtpy hiera files throw puppet 4 warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants