search

edbizarro / gitlab-ci-pipeline-php

:coffee: Docker images for test PHP applications with Gitlab CI (or any other CI platform!)
https://hub.docker.com/r/edbizarro/gitlab-ci-pipeline-php
MIT License
529 stars 167 forks source link

Add PHP 8.0 Support #113

Closed briandotdev closed 3 years ago

briandotdev commented 3 years ago

This PR adds PHP 8.0 support. I don't normally submit competing PRs however I had this 95% complete when the other PR came through and this is passing all tests whereas the other PR is silently failing the build of some modules that are not compatible with PHP 8. Significant changes are as follows:

Build/Test Level Changes

-Added 8.0 Dockerfiles (cli, chromium, fpm, alpine, alpine-lts) -Added 8.0 Goss test identical to 7.4 test but removing Cassandra extension that will not be supported in PHP 8

PHP Internal Changes

-XMLRPC is removed from PHP core per here -PHP 8 requires xdebug 3.0 per here -PHP 8 requires phpredis 5.3 or higher per here -Imagick extension supporting PHP 8 not yet released to PECL and installed from source per here -AMQP extension supporting PHP 8 not yet released to PECL and installed from source per here -XMLRPC extension supporting PHP 8 PECL package is broken and installed from source per here -Cassandra extension removed from PHP 8 builds due to no plans to support PHP 8 per here -Xdebug 3 changes config settings for enable code coverage per here

Note: At a future date a PR can be submitted to clean up/consolidate the build scripts once all these modules eventually make their way to PECL however building from source allows us to release a full feature PHP 8 image today.

Testing the PR

All of the new PHP 8 images were built and are passing all tests. I have also built and tested all PHP 7.2-7.4 images and ensured they are still passing all tests after modification of build scripts.

Additional Notes

The other PR bumps Node, NPM, and Composer to latest version however I felt this was outside the scope of this PR and have not included. I believe there is another PR already open for Composer or I am happy to submit additional PRs for these if needed in order to keep the scope of this change to PHP 8 only.

Pezhvak commented 3 years ago

so you just took my pr to create your own? wow dude fine

briandotdev commented 3 years ago

so you just took my pr to create your own? wow dude fine

Like I said, competing PRs is not something I normally do. As you can see, I put a lot of research into this and have been working it for a while. I did not download your source and copy your PR.

Your PR was silently failing the compile of multiple modules. I brought those up and your response was that I should do the testing and the container was working fine on your CI already. I was worried that if left uncorrected and your PR was adopted this would take the project backwards. So I gave you a few days to look into the issues I mentioned and then I offered up an alternative.

Nothing personal my friend. If you would like to incorporate the changes on the missing extensions I've got no issues closing this PR and supporting yours. 🙂

Pezhvak commented 3 years ago

@briandotdev you cannot test something that you haven't downloaded, after all it's open sourced, go ahead, it's all your. all i wanted in the end is to contribute to something that i once used. i appreciate your research and the time you put to gather this up. i just wished you did the same for me. best wishes

briandotdev commented 3 years ago

i appreciate your research and the time you put to gather this up. i just wished you did the same for me. best wishes

I did. That was my first attempt.

https://github.com/edbizarro/gitlab-ci-pipeline-php/pull/112#issuecomment-758837486 https://github.com/edbizarro/gitlab-ci-pipeline-php/pull/112#issuecomment-758900790 https://github.com/edbizarro/gitlab-ci-pipeline-php/pull/112#issuecomment-760272255

Pezhvak commented 3 years ago

@briandotdev lets get this over with and get you merged, i cannot teach you ethics, its just something that you either have or you don't have. just for the sake of respecting your reply, i have to add that i told you that i can fix it if there is a problem https://github.com/edbizarro/gitlab-ci-pipeline-php/pull/112#issuecomment-758886319 if you liked to fix my pr you could just ask for it, i would be more than happy to give you access to my forked repo so you could change what is needed, that's how contributing works, we build on top of our colleges work, copying and making something entirely yours is just something we don't do. i really can't keep myself engaged with this, have lots to do. unsubscribing from this.

polyskalov commented 3 years ago

So, @edbizarro, can you merge this PR? Really looking forward to this update

briandotdev commented 3 years ago

@wedi I've updated the Readme to reflect PHP 8.0. I've followed URL convention for badges but you guys will need to update those. I'm also seeing that some of the previous badges probably are needing an update also.

While updating the Readme had the realization that you have been depricating EOL PHP versions. PHP 7.2 could be removed as part of this PR or could be left to another PR in order to get this out the door. If that's the direction you want to go I'm happy to submit another PR or let someone else take a swing.

edbizarro commented 3 years ago

Thanks for the hard work guys!