Fixes #549: Unable to define a multi-value type #604
Conversation
|
Full marks for the attempt, but whenever I see code that makes me want to scrub my hands thoroughly, I start wondering about wholesale refactoring. Can we completely redo this defined type to make things easier to read and easier to maintain (regardless of Puppet 4 types)? |
|
loop in puppet 3 does not really exists. I have to use inline_templates. Or should I code a define or every type definition? |
juniorsysadmin
added
needs-feedback
|
@juniorsysadmin Any news about this? |
|
Hi @jkroepke, we dropped puppet3 in master. if you want you can migrate this PR to a puppet4 only codebase. |
|
ping @jkroepke |
|
@bastelfreak Looks like I forgot to send my question... What about epp templates? Can I use them instead erb files? |
|
yep sure! |
|
Fine. Then I will start it. |
jkroepke
force-pushed
the
multiple_types
branch
from
83ca9d0 to
52a921a
Compare
|
Okay, I have refactor this type. I'm using puppter 4 validations now. Additions Info:
|
|
Additionally, the new travis the tests confused me. While the spec tests running file, the Coverage tests are failed. Its related to my PR? |
|
@jkroepke we now run the tests in parallel, so the output might be a bit confusing. The original error is here: https://travis-ci.org/voxpupuli/puppet-collectd/jobs/200868572#L1360-L1366 |
jkroepke
force-pushed
the
multiple_types
branch
from
981b797 to
2513884
Compare
jkroepke
force-pushed
the
multiple_types
branch
from
72909fa to
98ad29d
Compare
|
Thanks. I squashed my commits and add a spec test for define multiple types. |
jkroepke
force-pushed
the
multiple_types
branch
from
98ad29d to
4c9e0d8
Compare
jkroepke
force-pushed
the
multiple_types
branch
from
4c9e0d8 to
d9508dc
Compare
| max => Variant[Numeric, Enum['U']], | ||
| ds_type => Enum['ABSOLUTE', 'COUNTER', 'DERIVE', 'GAUGE'], | ||
| ds_name => String, | ||
| }]] $types = [], |
There was a problem hiding this comment.
I think in this line is a bit too much whitespace?
There was a problem hiding this comment.
@bastelfreak I'm missing a guideline for the parameter layout w/ puppet data type definition. The reason for the much whitespace is to have all parameter variables in one vertical line. But I don't know, if it's fine.
There was a problem hiding this comment.
ah, now I understand the layout. I'm not aware of any current best practices for the layout, we will keep this as is.
|
@jkroepke thanks for the work! If I understand this correctly it is still backwardscompatible (then I can remove the label)? Plese check the one line with the whitespace, afterwards I can merge + do a release. |
|
@bastelfreak technically no, because ds_type must be an uppercase now. In 5.x release lowercase values are accepted, too. (And transfor to uppercase) |
Hi,
this pr implement #549 in a dirty way. It should be refactor if puppet 3 is dropped. (voxpupuli/plumbing#21 hopefully gets voted in the near feature... )
If you reject this because inline_template is not the state of the art I rewrite it to own defines for every type definition.