-
Notifications
You must be signed in to change notification settings - Fork 106
Removing firewalld snippet #4252
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
Conversation
The PR preview for 504cb23 is available at theforeman-foreman-documentation-preview-pr-4252.surge.sh No diff compared to the current base |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style-wise LGTM, but seeing that the snippet is used fived times proves that is is being reused.
I am OK either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is already part of #4210 where it's integrated into the review of our firewall procedures. Please don't merge this.
As Maximilian already pointed out, the snippet is actually reused multiple times so I admit I don't understand this statement. In #4210, I'm removing the snippet file while also re-evaluating whether the information it contains is useful in some of the procedures. In the procedures where I don't think it is, I propose to remove it entirely. That PR is available for feedback and a conversation over the future of the snippet. |
I didn't express myself very clearly. This snippet is being reused. I would say this is an example of a 'frivolous snippet. A snippet's usefulness is questionable when the content is a single step or sentence, unless the wording must be extremely accurate (example: legal note). If we create snippets for every possible text that might be reused, we end up with greater maintenance problems in the long run:
In my experience, snippets are useful for complex content (like config files), which should be maintained in only one place, and for legal notices (technology preview), where the wording must be uniform and easy to update. Since there is another PR in progress on firewall requirements, I will close this PR. |
What changes are you introducing?
Removing firewalld snippet
Why are you introducing these changes? (Explanation, links to references, issues, etc.)
Snippet is only 1 sentence and unlikely to be reused.
Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)
No tech review or testing required
Contributor checklists
Please cherry-pick my commits into:
Review checklists
Tech review (performed by an Engineer who did not author the PR; can be skipped if tech review is unnecessary):
Style review (by a Technical Writer who did not author the PR):