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 simple config for security policy resource #1044

Merged
merged 1 commit into from
Sep 12, 2016

Conversation

chris-rock
Copy link
Contributor

  • Uses the simple config parser for security_policy resource
  • harmonizes the api with ssh_config
  • security_policy.content to retrieve the plain file content
  • security_policy.params to retrieve the parsed parameters

read_content
end

def params(*opts)
Copy link
Contributor

Choose a reason for hiding this comment

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

A number of the resources include a params function that feels similar. Might be nice to pull this out into a module where the behavior can be tested once.

It's a bit strange that some of the resources take arguments for their params method while others return a hash.

Fine for now, just figured it was worth pointing out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, resources need some harmonization here and it would be nice to extract some basics. This improvement makes sense to me...

@stevendanna
Copy link
Contributor

This appears to be following the conventions of the other resources we have. 👍

@chris-rock chris-rock merged commit 4269cb3 into master Sep 12, 2016
@chris-rock chris-rock deleted the chris-rock/security_policy branch September 12, 2016 10:31
@chris-rock chris-rock modified the milestone: 0.34.0 Sep 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Improves an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants