-
Notifications
You must be signed in to change notification settings - Fork 231
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 variables for Ubuntu 20.04 #215
base: master
Are you sure you want to change the base?
Conversation
Best reviewed: commit by commit
Optimal code review plan
|
Thanks for this PR, @bramd -- hopefully @sticky-note will get a chance to review this soon. Generally looks fine (other than the fact the |
I had problems with this too. The commit looks good to me, although there might be better solutions for map.jinja. However, can this pull-request still be accepted? |
@crosscodr Can you confirm that these changes work for you? |
Almost, but a few packages are not available in the official ubuntu-sources: I also noticed, that this part 'SQL': { 'sql.safe_mode': 'Off' }, can be omitted, hence it is removed since php7.2 (https://www.php.net/manual/de/ini.core.php#ini.sql.safe-mode). This are my findings so far. There may be other things to optimise, but these are the main problems I can see. |
Thanks @crosscodr. @bramd can you use the last comment to update this PR? |
Or maybe we should review and enhance #214. |
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
Apologies for the delay, @sticky-note -- the CI situation is still taking up a lot of my attention. There are a number of ways to approach this but I don't want to write a whole essay here! The easiest suggestion is to simply open up another PR, using the same commit again (with your amendments). That way, the authoring credit still goes back to the original author. Rebasing would be good because I just upgraded the CI matrix here to include the latest platforms including Lines 136 to 149 in daa4c9e
Any changes you make will have to be reflected in the relevant |
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
Describe the changes you're proposing
The formula did not work out of the box on a fresh Ubuntu 20.04 system. Some PHP 7.2 paths and package names were hardcoded, where Ubuntu 20.04 repositories contain PHP 7.4.
Pillar / config required to test the proposed changes
Debug log showing how the proposed changes work
Documentation checklist
README
(e.g.Available states
).pillar.example
.Testing checklist
state_top
).Additional context