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.

Sign up for GitHub

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

Jump to bottom

Manage journald service and configuration #89

Merged
merged 1 commit into from
Oct 23, 2018
Merged

Manage journald service and configuration #89

merged 1 commit into from
Oct 23, 2018

Conversation

treydock
Copy link
Contributor

No description provided.

bastelfreak
bastelfreak reviewed Oct 17, 2018
View reviewed changes
manifests/init.pp Outdated Show resolved Hide resolved
@bastelfreak bastelfreak added the enhancement New feature or request label Oct 17, 2018
@trevor-vaughan
Copy link
Contributor

No type validation? Are the journald options just too varied?

@bastelfreak
Copy link
Member

Hey @treydock, thanks for the work! I did an inline suggestion. Can you also document it in the README.md?

@treydock
Copy link
Contributor Author

@trevor-vaughan I suppose a custom type where the validation is done on a per-key basis would make sense, just not sure how it would survive for varied versions of systemd. This would require subtypes to handle the varied units that are allowed for different keys.

@bastelfreak Updated README.

@trevor-vaughan
Copy link
Contributor

@treydock Yeah, I get it. The config hashes just always feel like a 'slightly better than just flopping down raw content' approach to life without some sort of validation :-|

@raphink
Copy link
Member

raphink commented Oct 18, 2018

Agreed @trevor-vaughan. I usually like to provide a Data Type for the hash options to limit the scope of input data.

@treydock
Copy link
Contributor Author

I'll add a custom data type.

@bastelfreak bastelfreak added the needs-work not ready to merge just yet label Oct 18, 2018
@treydock
Copy link
Contributor Author

Added custom data type but looks like #14 did same thing as this pull request. Should I just make this pull request the addition of data type?

@raphink
Copy link
Member

raphink commented Oct 23, 2018

Ah right, I'm very sorry about that @treydock I hadn't noticed… Sure, the data type would still be a nice addition.

@ekohl
Copy link
Member

ekohl commented Oct 23, 2018

https://rspec-puppet.com/documentation/type_aliases/ can be used to test type aliases for various input.

@raphink
Copy link
Member

raphink commented Oct 23, 2018

Very nice!

@treydock
Copy link
Contributor Author

New data type added

@raphink raphink merged commit f6929de into voxpupuli:master Oct 23, 2018
@treydock treydock deleted the journald branch October 23, 2018 15:39
kajinamit pushed a commit to kajinamit/puppet-systemd that referenced this pull request Mar 25, 2023
@saz
Merge pull request voxpupuli#89 from JAORMX/rename-rubocop
c9b5203 
Fix .rubocop.yml issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bastelfreak bastelfreak bastelfreak left review comments

@raphink raphink raphink approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request needs-work not ready to merge just yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@treydock @trevor-vaughan @bastelfreak @raphink @ekohl