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 RHEL7 SELinux support for new service_name_v6 param, subsequently fix puppet lint error #671

Merged
merged 1 commit into from Dec 28, 2016

Conversation

wilson208
Copy link
Contributor

@wilson208 wilson208 commented Dec 28, 2016

This originally started as a fix for a puppet lint issue in CI, where the $seluser variable had been removed from the firewall::linux::redhat class as seluser property for the File["/etc/sysconfig/${service_name}"] resource is now set in a case statement.

When a separate PR was merged adding the 'service_name_v6' param, it still used the $seluser variable to set the seluser property of the File["/etc/sysconfig/${service_name_v6}"] resource.

When both these PR's were merged they then caused a puppet lint error in CI.

@ferventcoder
Copy link
Contributor

I'd prefer to see the additional fields be added as a separate commit than the lint fixes.

@wilson208
Copy link
Contributor Author

@ferventcoder I began with solving the lint issue. However I noticed the lint issue was because the $seluser variable had been removed and the result of solving the lint issue is this PR. I didn't add the additional service_name_v6 param, it was added in PR #641, and the variable $seluser was removed in PR #658. The PR for 641 was created in June and not merged until last week, which is why this was missed when david made his changes in #658 at the end of november.

@ferventcoder
Copy link
Contributor

Now I better understand what is going on after looking closer.

@ferventcoder
Copy link
Contributor

Minor nit* - would love to see the git commit message summary be around 50 chars with a message body. I looked over history for this repo and didn't find any long summaries.

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

3 participants