Basic OpenResty support #268
Conversation
|
Best reviewed: commit by commit
Optimal code review plan (1 warning)
|
Thanks for your feedback. Tried to follow the guideline regarding having two distinct |
|
I was just passing by (I don't maintain this repo) but yes, maybe I should have reviewed #267 first. Anyway LGTM. |
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'.
Allow to install OpenResty from their official repository by introducing 'install_from_openresty' pillar.
faudebert
force-pushed
the
upstream/openresty
branch
from
0769e3e to
f455b72
Compare
|
Rebased on |
|
@sticky-note Would you mind looking at this and #267? Please let us know if time is short and we'll look at some way of getting these reviewed. @faudebert From a reviewing perspective, smaller PRs are much easier to review, so usually get done much quicker. Even single-line changes can have significant impacts, the further we extend, the more cautious we get. We'll probably have to do that here where we leave these two PRs in place and then work through the proposals in smaller blocks in further PRs. For example, we've already got a new standard emerging for
We've had quite a few discussions about this in the Formulas' working group meetings, such as here: We're also implementing a new verification standard to use before refactoring any
It would be important to bring those in here first before refactoring These are just some points to help you see where we are currently. Please feel free to continue the discussion. We value your contribution and feedback, it just requires a structured approach to review. |
|
I'll look at this one once #267 is merged |
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
This PR relies on #267 which should be thus considered before. In this PR only introduce the last 2 commits.
Describe the changes you're proposing
This pull-request attempts to reintroduce (basic) OpenResty support within the NGINX formula.
Pillar / config required to test the proposed changes
Set
nginx:install_from_openresty: Trueand use the formula as usual.Debug log showing how the proposed changes work
Having set
nginx:install_from_openresty: True, check apply.log to seesalt-calloutput on a clean Debian Stretch host.Documentation checklist
README(e.g.Available states).pillar.example.Testing checklist
state_top).Additional context
I don't know why previous OpenResty support was dropped. I hope this might be of any interest.