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: Remove Craft CMS PHP version override #5805

Merged
merged 4 commits into from Apr 4, 2024

Conversation

AugustMiller
Copy link
Contributor

@AugustMiller AugustMiller commented Feb 8, 2024

The Issue

We have just tagged Craft 5.0.0-beta.1, and with that, are requiring PHP 8.2. The craftcms project type currently uses PHP 8.1, which will not satisfy the requirements.

Our upgrade guide covers switching PHP versions using the config command (ddev config --php-version=8.2), but we would like for all projects moving forward to use the most recent/default DDEV PHP version (8.2)—with which Craft 4 is also fully compatible.

The current database version (MySQL 8.0) is still our recommended engine, so no changes have been made to that configuration.

How This PR Solves The Issue

The craftCmsConfigOverrideAction() function no longer overrides the PHP version used when creating new projects.

Manual Testing Instructions

Create a new DDEV project:

mkdir craft-five
cd craft-five
ddev config --project-type=craftcms --docroot=web
ddev composer create -y --no-scripts craftcms/craft
ddev craft install

If Craft doesn't complain about the PHP version (or emit a lower-level language error), you’re all set!

Automated Testing Overview

No automated tests have been added or updated.

Related Issue Link(s)

Release/Deployment Notes

  • Craft CMS project types now use DDEV's default PHP version.

Copy link

github-actions bot commented Feb 8, 2024

@rfay
Copy link
Member

rfay commented Feb 8, 2024

Hmm, that line doesn't probably need to be there in the first place, since 8.1 is default.

Also, 8.2 will be the default in DDEV v1.23, coming as next release probably in a couple of months.

I'm still OK with this, just things to consider. It would also be OK to update the quickstart and add it there, which would make it effective immediately in the "latest" docs.

@rfay
Copy link
Member

rfay commented Feb 8, 2024

And of course, thanks so much for watching these things and keeping up the maintenance!

@rfay rfay changed the title Use PHP 8.2 as craftcms project type default fix: Use PHP 8.2 as craftcms project type default Feb 8, 2024
@rfay rfay changed the title fix: Use PHP 8.2 as craftcms project type default fix: Use PHP 8.2 as craftcms PHP version default Feb 8, 2024
@github-actions github-actions bot added the bugfix label Feb 8, 2024
@AugustMiller
Copy link
Contributor Author

Noted! I'll talk with the crew and revise or close. We are currently planning on a 6–8 week beta period, so the docs route may be preferable, in the event our timelines don't quite line up. ✌️

@rfay
Copy link
Member

rfay commented Feb 8, 2024

Still, there's nothing wrong with your change here, does no harm, just requires future maintenance as time moves along and PHP versions move along. And you do a great job keeping track, so should be fine anyway.

@rfay
Copy link
Member

rfay commented Feb 14, 2024

PHP 8.2 default:

But I note that Craft has a bunch of overrides that set it to 8.1, so we'll definitely want to update things here. The two ways of doing it would be to let it track the defaults (not do overrides) or just update the overrides.

Do we also want to force composer_version: 2.7 ?

@stasadev stasadev added this to the v1.23.0 milestone Feb 26, 2024
@rfay
Copy link
Member

rfay commented Feb 26, 2024

@AugustMiller this needs a restart now, since PHP 8.2 is now default, etc. Do you want to continue with this one or start over again?

@rfay
Copy link
Member

rfay commented Mar 5, 2024

Closing for now, happy to reopen, happy to continue the conversation here or elsewhere. Definitely want this to be up-to-date for Craft 5 and DDEV v1.23.0

@rfay rfay closed this Mar 5, 2024
@rfay
Copy link
Member

rfay commented Apr 2, 2024

We should finish this! The new release is almost ready. Reopening, we'll have to get it done.

@rfay rfay reopened this Apr 2, 2024
@AugustMiller AugustMiller changed the title fix: Use PHP 8.2 as craftcms PHP version default fix: Remove Craft CMS PHP version override Apr 2, 2024
@AugustMiller
Copy link
Contributor Author

As we're out of beta now, I've removed the version override so that we can take advantage of DDEV's default, moving forward. This way, when DDEV is ready to move to 8.3 as the default, Craft projects will automatically get the latest version (rather than needlessly locking users to an older release).

Thanks for reviving, Randy! Holler if you need anything else from us. 💞

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Fixed the broken test, after that it looks fine to me. Thanks!

@rfay rfay marked this pull request as ready for review April 3, 2024 22:51
@rfay rfay requested review from a team as code owners April 3, 2024 22:51
@rfay rfay merged commit 07b12e5 into ddev:master Apr 4, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants