Currently the installer has:
if (version_compare(PHP_VERSION, '7.3.0') < 0) {
+ print 'Your PHP installation is too old. Drupal requires at least PHP 7.3.0. See <a href="http://php.net/supported-versions.php">PHP\'s version support documentation</a> and the <a href="https://www.drupal.org/docs/system-requirements/php-requirements">Drupal PHP requirements</a> page for more information.';
exit;
}
However, once #2917655: [9.4.x only] Drop official PHP 7.3 support in Drupal 9.4, it could be bad user experience for them to then upgrade to PHP 7.4, then get a nearly identical message that PHP 7.4 is too old.
If they read the second link in detail, they'd know not to pick PHP 7.4 either. But a lot of people don't read docs.
'Your PHP installation is too old. Refer to the Drupal PHP requirements for the currently recommended PHP version for this release. See PHP\'s version support documentation for more information on PHP's own support schedule.';
Before
After
N/A
N/A
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #27 | Screen Shot 2022-12-23 at 10.37.01 AM.png | 80.76 KB | sergiogsanchez |
| #16 | After.png | 18.25 KB | quietone |
| #16 | Before.png | 14.16 KB | quietone |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !3120changes, plain diff MR !2054
Comments
Comment #2
eelkeblokFixed some typos.
Comment #3
eelkeblokHow about we drop the "Drupal requires at least PHP 7.3.0." to stop any confusion arising and make it (even) more obvious the links contain the information needed:
The documentation page also says PHP 7.3 is supported, but with a warning icon indicating it is nit *recommended*, so I guess that's the nuance that is needed.
Comment #4
xjmComment #5
xjm@eelkeblok, that's a worthwhile idea. 👍 Next step then is to create an MR and demo it for UX review, I think.
Comment #7
eelkeblokOK, step 1. I bet there are one or two tests that will not like this.
FWIW, I have no idea how to demo it for UX review...
Comment #8
xjmThanks @eelkeblok!
I do one of two things:
ifstatement around this to be a higher PHP version than the one I have installed.And then run the installer.
Comment #9
xjmNW for points on the MR review, thanks!
Comment #10
eelkeblokSorry, I meant the "soft" part of that, how to get it in front of the UX team. I actually tested the code much like you suggested :)
Comment #12
eelkeblokThanks. I pushed another commit that restores the "Your PHP version is too old.", I think that was removed unintentionally. Without it, I think the message is not explicit enough.
Comment #13
eelkeblokComment #16
quietone commentedJust making the recommended change and updating the IS.
Comment #17
quietone commentedMy changes to the IS were lost - trying again.
Comment #19
smustgrave commentedThe change looks good to me and make sense +1
Think we missed the 9.4 target. Should this go into 9.5 or be pushed to 10.1? If pushed @quietone can you update the MR please?
Comment #20
xjmUI changes like this are minor-only, so the issue is correctly filed against 10.1.x. Thanks!
Comment #21
smustgrave commentedMoving to NW to open a new MR for 10.1
Finding quickly updating an MR from 9.x to 10.x is causing a headache so disregard #19 comment.
Comment #23
smustgrave commentedOpened up a D10 branch but think that takes me out of the review process.
Comment #25
xjmI closed the old 9.5.x MR.
Comment #26
xjmComment #27
sergiogsanchez commentedI tested patch #3120 on a PHP 7.3 environment; it works as expected, and the links are correctly pointed to the Drupal and PHP documentation.
I checked the root path, disabling the composer platform check and going directly to /core/install.php
Comment #31
xjmThanks @sergiogsanchez!
I provided some review that resulted in the current text, but it was based off @eelkeblok's suggestion, and has been reviewed by several other people since, so I feel comfortable committing this. Committed to 10.1.x.
As I indicated in #20, this is a UI change. It won't affect existing sites, because it's the installer. Sometimes, changes to the installer can be disruptive; however, in this case, someone reaching this page wouldn't get anywhere anyway. Furthermore, we're not even breaking translated strings. So, contrary to my previous statement, I did also go ahead and backport this to 10.0.x with a cherry-pick, and to 9.5.x using the old MR for the backport version.
Thanks everyone!