Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

PHP 7.4, 8.0 and 8.1 #27

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

PHP 7.4, 8.0 and 8.1 #27

wants to merge 3 commits into from

Conversation

claytoncollie
Copy link

Adds support for PHP 7.4, 8.0 and 8.1

Closes #23 #24 #25

How to test the Change

I don't know how to test this change.

Changelog Entry

Added - PHP 7.4
Added - PHP 8.0
Added - PHP 8.1

Credits

Props @claytoncollie

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@claytoncollie
Copy link
Author

Tagging @darylldoyle here as this pertains to internal tooling. I made this PR because I was wpsnapshots was failing when there was a union type in my codebase (php 8). I think wpsnapshots is using a docker image with PHP 7.4

@darylldoyle
Copy link

@claytoncollie it seems that wp-local-docker doesn't use these images, it uses the ones over at https://github.com/10up/wp-php-fpm-dev. I'm actually unsure what purpose these images or even this repo serve.

The https://github.com/10up/wp-php-fpm-dev repo already has images for everything up to 8.2; they're just not included in wp-local-docker. There's a PR to add support for them here, it's also possible to manually upgrade by updating the image in your docker-compose.yml file within a project. E.G.

  # ...
  phpfpm:
    image: '10up/wp-php-fpm-dev:8.2-ubuntu'
  # ...

@claytoncollie
Copy link
Author

@darylldoyle My problem is not with wp local docker but with wpsnapshot. I have my project's container running PHP8 and am using union types on a function like this function my_function_with_two_return_types() : array| bool {}. The project works fine but when I run wpsnapshot create on this project, the command throws an error. I think it is because wpsnapshot is spinning up its own container to run the job.

@darylldoyle
Copy link

Thanks @claytoncollie, I see what you mean! I feel like https://github.com/10up/wpsnapshots/blob/cf28870fcf0fa3cc3f3ad07c6bce74c713bf587b/docker/Dockerfile#LL1C6-L1C17 should be updated to use 10up/wp-php-fpm-dev rather than 10up/wp-php-fpm, since those are the new docker images 🤔

@dustinrue or @tylercherpak do you have any thoughts around this?

@darylldoyle
Copy link

Thanks @claytoncollie. It seems that the wpsnapshots docker image is no longer sourced from this repo, see #20 for details.

It's now built via a Github action from the https://github.com/10up/wpsnapshots repo. It looks like this file will need updating to use a more modern image. Likely one of the images in https://hub.docker.com/r/10up/wp-php-fpm-dev/ will be the best, since that has 8.* images. I need to dig in and see if that needs any modifications to the setup to work since those images are different.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for PHP 8.1 Add support for PHP 8.0 Add support for PHP 7.4
2 participants