-
Notifications
You must be signed in to change notification settings - Fork 417
fix(snippet-file-name): removed appending of ".conf" for snippets files #240
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
fix(snippet-file-name): removed appending of ".conf" for snippets files #240
Conversation
noelmcloughlin
left a comment
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.
Thanks @sticky-note
I cannot see any technical issues with the code.
Could you update the PR description to elaborate on what problem is being fixed?
|
Hello, I think this PR introduces a 'BREAKING CHANGE' because people must adapt their pillar. This must be explicitely stated in the commit message, can you update it? |
6150a3c to
5553c21
Compare
|
@daks @noelmcloughlin updated, is that ok? |
|
Not sure the commit 'BREAKING CHANGE' message is formatted correctly https://github.com/saltstack-formulas/nginx-formula/blob/master/docs/CONTRIBUTING.rst#use-breaking-change-to-trigger-a-major-version-change otherwise good for me, nice catch @sticky-note :) |
5553c21 to
aa87721
Compare
|
@daks If I read correctly, it is ok now. Sorry for multiple editing. |
noelmcloughlin
left a comment
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.
Thanks @sticky-note for the hard work and responsiveness & @daks for reviewing
|
Thanks to everyone for this PR. For reference, this PR links back to #238 (comment). |
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
(...) for snippets filename. BREAKING CHANGE: Users have to modify their pillar according to this commit. Users MUST append '.conf' for their existing managed snippets.
Since introduction of TOFS in this formula, we can retrieve file that are not described by pillar. TOFS will generate source like the following:
However, we have not same behavior between snippets file and
server_configs.Actually formula, append
.confas an extension for all snippets.If we include
snippets/*.confin the main nginx.conf for example, it is not always the case that we desire to include all snippets.This PR propose to remove the appending of '.conf' for snippets files and permit to choose file name and extension for snippets with snippet name.