- Issue created by @oldeb
- First commit to issue fork.
- ๐บ๐ธUnited States trackleft2 Tucson, AZ ๐บ๐ธ
My merge requests adds back support for PHP 7.4
It looks like the issue was caused by the trailing comma on this line:
https://git.drupalcode.org/project/environment_indicator/-/merge_request...
This syntax is valid in newer PHP versions, but breaks on PHP 7.4, which the module currently supports.To help catch similar issues, Iโve added a PHPUnit test to lint PHP files across supported versions:
https://git.drupalcode.org/project/environment_indicator/-/merge_request...While this wonโt catch the trailing comma specifically (as PHP 7.4 is no longer available in the GitLab runner environment), it sets us up for stricter compatibility checks going forward. Iโd recommend holding off on using syntax that requires PHP 8.0+ until the module drops support for older versions in a future minor release.
I am also adding this issue to the ๐ฑ [META] Release Plan for Environment Indicator Patch Release 4.0.21 Needs review
- ๐จ๐ฆCanada Liam Morland Ontario, CA ๐จ๐ฆ
If changes like this are going to be made, it makes it very urgent to get a branch which is aimed at currently-supported versions of Drupal. This change introduces a coding standards error. Contributing to this module will become more difficult because people will need to remember to write in an old version of PHP. remembering what changes have been made and what coding standard issues to ignore.
- ๐บ๐ธUnited States trackleft2 Tucson, AZ ๐บ๐ธ
I agree that maintaining compatibility with PHP 7.4 makes contributions trickier, and thereโs value in targeting modern environments. That said, this patch is specifically scoped to fix a regression introduced in a patch release (4.0.18โ4.0.21) that broke sites still running PHP 7.4 โ without any documented change in requirements.
To avoid repeating that mistake, Iโd suggest the following sequence:
- Merge this fix to restore compatibility in 4.x and ensure sites on PHP 7.4 can safely upgrade within the same major version.
- Create a final 4.0.x release that:
- Maintains support for Drupal 9.3 through 11.
- Avoids changes that break backward compatibility for supported environments.
- After that, address the syntax issue in ๐ฑ Drop support for versions of Drupal older than 10.3 Active , which also drops support for Drupal core versions earlier than 10.3.
- Then create a 4.1.0-beta1 minor release
Weโd appreciate review on any of the โNeeds reviewโ issues blocking the final 4.0.x release:
๐ฑ [META] Release Plan for Environment Indicator Patch Release 4.0.21 Needs review or ๐ฑ Draft: Release Plan for Environment Indicator 4.1.0-beta1 Active - ๐จ๐ฆCanada Liam Morland Ontario, CA ๐จ๐ฆ
I think it would be reasonable to fix just the regression and then move on to 4.1.x.
- ๐บ๐ธUnited States trackleft2 Tucson, AZ ๐บ๐ธ
I've updated the PHPUnit tests so that they pass in all of the gitlab-ci jobs.
While updating the PHPUnit Test I was testing the module using the combination of PHP 8.3 and Drupal Core 11.0.13 as well.
- ๐บ๐ธUnited States trackleft2 Tucson, AZ ๐บ๐ธ
Also, it appears that all of the gitlab-ci tests are currently passing due to this issue: ๐ฌ Validate stage may use outdated artifact, leading to flaky test results Active