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

Add table plugin #826

Merged
merged 1 commit into from
Jul 9, 2018
Merged

Add table plugin #826

merged 1 commit into from
Jul 9, 2018

Conversation

sileht
Copy link
Contributor

@sileht sileht commented Jul 9, 2018

No description provided.

class collectd::plugin::table (
Enum['present', 'absent'] $ensure = 'present',
Integer $order = 10,
Hash[String, Struct[{
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this awesome and detailed datatype! Can you please abstract it into its own datatype in https://github.com/voxpupuli/puppet-collectd/tree/master/types ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, awesome I was not aware we can declare type there :)

}], 1]
}], 1] $tables,
) {
include ::collectd
Copy link
Member

Choose a reason for hiding this comment

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

please don't add the leading :: infront of a class name. This isn't needed anymore since we don't support puppet 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, old habbit.

@@ -0,0 +1,27 @@
<Plugin "table">
Copy link
Member

Choose a reason for hiding this comment

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

yey epp template \o/

<Plugin "table">
<% $collectd::plugin::table::tables.each |$path, $table| { -%>
<Table "<%= $path %>">
<% unless $table['plugin'] =~ Undef { -%>
Copy link
Member

Choose a reason for hiding this comment

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

if $table['plugin'] is the same as unless $table['plugin'] =~ Undef. I think the if notation is shorter and better to read?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if plugin is set "", the if version will evaluation to false while the unless to true.

Copy link
Member

Choose a reason for hiding this comment

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

ah right, good catch.

@bastelfreak bastelfreak added backwards-incompatible needs-work not ready to merge just yet enhancement New feature or request and removed backwards-incompatible labels Jul 9, 2018
@bastelfreak
Copy link
Member

Thanks for your work @sileht! can you please take a look at the inline comments I made?

@bastelfreak bastelfreak merged commit 3de5000 into voxpupuli:master Jul 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-work not ready to merge just yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants