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

Allow redact to take defines as well as classes #17

Merged
merged 1 commit into from Jun 27, 2017

Conversation

ross-w
Copy link

@ross-w ross-w commented Jun 21, 2017

Hi there

Forgive me; I'm a complete newbie at this level of Puppet internals. Is there any reason the redact function can't be modified in this way to work with defines as well as classes?

It /appears/ to work for our use case, but I can't be sure I'm not missing something really obvious or breaking something rather important. Comments and/or rejection welcome.

@ross-w
Copy link
Author

ross-w commented Jun 26, 2017

Hi @binford2k tell me to go away if you think I'm being a nag. I would be interested to know what you think of this, if there's something else I've not thought of or missed, etc.

@binford2k
Copy link
Member

@ross-w or @sammcj what validation steps did you take? As long as you've tested on classes & defines both, I'll totally do some more stringent testing and merge this.

@ross-w
Copy link
Author

ross-w commented Jun 27, 2017

@binford2k tested on one define, two defines, one class. That said I could have missed something obvious that makes the whole thing pointless or breaks something else important that I haven't noticed. But from my testing it /seems/ to work...

@binford2k binford2k merged commit d16d759 into puppetlabs:master Jun 27, 2017
@binford2k
Copy link
Member

Added some tests and merged, c6a67cf

@sammcj
Copy link

sammcj commented Jun 28, 2017

Thanks for that @binford2k, do you mind tagging the release so we can add the requirement to https://github.com/sammcj/puppet-luks ? Thanks,

@binford2k
Copy link
Member

@sammcj if it's a priority, I'd be happy to make a minor release. There are a couple more updates I wanted to make, but they can wait.

@sammcj
Copy link

sammcj commented Jun 30, 2017

No problems, the tag can wait :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants