Skip to content
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 small bug in run.sh + drop support for php < 7.2 + update CHANGELOG #167

Merged
merged 3 commits into from
Dec 22, 2019

Conversation

dsifford
Copy link
Collaborator

@dsifford dsifford commented Dec 7, 2019

This PR fixes a small bug in run.sh resulting from a typo in a find flag.

Additionally, this PR drops support for PHP < 7.2 to align with php

Finally, the build script was updated to remove useless cruft and the CHANGELOG was brought up to date.

@dsifford dsifford requested a review from karellm December 7, 2019 20:27
@@ -21,7 +19,7 @@ for php_version in "${php_versions[@]}"; do
-t "visiblevc/wordpress:latest" \
-t "visiblevc/wordpress:latest-php${php_version}" \
-t "visiblevc/wordpress:$npm_package_version-php${php_version}" \
"$dockerfile_dir"
.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed since running using npm will ensure that the $PWD is the root of the repo.

@dsifford
Copy link
Collaborator Author

dsifford commented Dec 7, 2019

Okay, so this got a bit more complicated...

It appears the configuration for php7.4 extensions has diverged from versions below it, so we're no longer able to share dockerfiles anymore.

See here for an example of this. There are other such cases as well.

So have a look at this and let me know what you think @karellm

Copy link
Collaborator

@karellm karellm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have limited time at the moment to review and test this fully. I did read the code and it looks good to me though. I trust you to merge and do the tests needed. Thanks for maintaining this project so actively!

@dsifford
Copy link
Collaborator Author

Sorry for the delay. Sounds good.

We've been using this at my workplace so I'm able to find and correct issues relatively quickly. I'll be sure to do so if we notice any probs with this.

@dsifford dsifford merged commit 6126590 into master Dec 22, 2019
@dsifford dsifford deleted the update-images-and-fix-small-bug branch December 22, 2019 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants