-
Notifications
You must be signed in to change notification settings - Fork 417
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
More flexible & generic config #267
base: master
Are you sure you want to change the base?
More flexible & generic config #267
Conversation
Best reviewed: commit by commit
Optimal code review plan (1 warning)
|
5a9f6e0
to
19a6656
Compare
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.
LGTM, can be merged before #268. thanks @faudebert
@sticky-note Would you mind reviewing this PR? |
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.
The rest looks good to me but haven't checked in details.
@@ -357,3 +357,7 @@ nginx: | |||
- alt_nginx.service | |||
nginx_snippet_file_managed: | |||
- alt_server.conf | |||
|
|||
# Configure formula pillar namespace | |||
pillar: |
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 pillar:namespace
parameter and use seems to be something new accross all community formulas, but it may be a solution to a problem @myii @sticky-note are discussing.
This is already done mostly everywhere and covers few remaining locations.
Allow to rename the meta-state without having to change substates references. This might be useful one wants to use this formula with a (renamed) fork within the same environment.
This might be useful if one wants to use this formula with a (renamed) fork within the same environment. Note this also permits to override pillar namespace by defining '{meta-state name}:pillar:namespace: str'.
02e45d4
to
5d1addc
Compare
Rebased on Let me know if I can do anything to help with the merge process. |
@daks @faudebert @noelmcloughlin Sorry for latency. |
PR progress checklist (to be filled in by reviewers)
What type of PR is this?
Primary type
[build]
Changes related to the build system[chore]
Changes to the build process or auxiliary tools and libraries such as documentation generation[ci]
Changes to the continuous integration configuration[feat]
A new feature[fix]
A bug fix[perf]
A code change that improves performance[refactor]
A code change that neither fixes a bug nor adds a feature[revert]
A change used to revert a previous commit[style]
Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)Secondary type
[docs]
Documentation changes[test]
Adding missing or correcting existing testsDoes this PR introduce a
BREAKING CHANGE
?No.
Related issues and/or pull requests
None.
Describe the changes you're proposing
This pull-request makes (hopefully) better use of
jinja.map
and allows to seamlessly rename meta-state. While not useful per se for the official repository, renaming permits easier reusability when forking the formula (ex. you want to use both original and forked formulas in your environment).Pillar / config required to test the proposed changes
No specific pillar is required. If my changes are correct, you should not see any visible changes when applying your state.
If interested, you could test meta-state renaming (ex.
mv nginx-formula/{nginx,mynginx}
). Considering no other changes have been made, you should be able to usemynginx
meta-state as drop-in replacement fornginx
. Also note that pillar namespace defaults to the meta-state name (i.e.mynginx
in that example).Optionnaly, you can customize the formula pillar namespace by setting
{meta-state-name}:pillar:namespace
pillar (ex.mynginx:pillar:namespace: mynginx2
).Debug log showing how the proposed changes work
None for now.
Documentation checklist
README
(e.g.Available states
).pillar.example
.Testing checklist
state_top
).Additional context