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

Use some more puppet 4 features to reduce code #1058

Merged
merged 4 commits into from
Apr 12, 2017
Merged

Use some more puppet 4 features to reduce code #1058

merged 4 commits into from
Apr 12, 2017

Conversation

igalic
Copy link
Contributor

@igalic igalic commented Mar 28, 2017

and increase readability!

  • Use contain instead of include + anchor
  • Use type alias in place over overly long inline Type Declaration

@@ -172,8 +172,8 @@
manage_repo => $manage_repo,
}
Copy link
Member

Choose a reason for hiding this comment

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

contain '::nginx::package' needed too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, no, as you can see, it's instantiated in resource format for reasons not entirely clear to me…

Copy link
Member

Choose a reason for hiding this comment

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

@igalic You can, (and I think should), still contain it.

To contain classes that are declared with the resource-like declaration syntax, use the contain function after declaring the class
https://docs.puppet.com/puppet/4.9/lang_containment.html

@bastelfreak
Copy link
Member

@igalic can you check the failed tests? We can probably just drop them.

@igalic
Copy link
Contributor Author

igalic commented Mar 31, 2017

funnily enough, the failed tests were because of the removed anchors.
i'll look into incorporating @alexjfisher's idea to use contain for nginx::package, too.

@wyardley
Copy link
Collaborator

+1 for this (even if we have to adjust / kills some tests). This should make things a lot more readable.

Agree on the 'fixme' bits, that pattern is really redundant and should probably have a function that handles it in a generic way. If someone comes up with the base pattern, I could probably help converting some of the templates.

igalic added 4 commits April 12, 2017 12:01
i think it's very much on point, that the first line of this commit
message is longer than the recommended 59 characters.
puppet 3.7 already had this down, and we're requiring 4.6
worse yet, it's in ruby templates, rather than in puppet
@bastelfreak
Copy link
Member

@igalic thanks!

@bastelfreak bastelfreak merged commit b47c3d0 into voxpupuli:master Apr 12, 2017
@igalic igalic deleted the type/alias branch April 12, 2017 11:02
Rubueno pushed a commit to Rubueno/puppet-nginx that referenced this pull request Oct 19, 2020
Use some more puppet 4 features to reduce code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants