Skip to content

Require PHP 8.3 #6889

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

Merged
merged 1 commit into from
Apr 6, 2025
Merged

Require PHP 8.3 #6889

merged 1 commit into from
Apr 6, 2025

Conversation

morozov
Copy link
Member

@morozov morozov commented Apr 4, 2025

PHP 8.1 and 8.2 are no longer supported by the community. PHPUnit 12 also requires PHP 8.3 or newer (see #6888).

The code/phpDoc changes are needed to address the following coding standard violations:

FILE: src/Driver/PgSQL/Exception/UnexpectedValue.php
------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------
 23 | ERROR | [x] Method \Doctrine\DBAL\Driver\PgSQL\Exception\UnexpectedValue::getSQLState() has useless
    |       |     @return annotation. (SlevomatCodingStandard.TypeHints.ReturnTypeHint.UselessAnnotation)
------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------


FILE: src/Configuration.php
------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------
 137 | ERROR | [x] Method \Doctrine\DBAL\Configuration::getDisableTypeComments() has useless @return
     |       |     annotation. (SlevomatCodingStandard.TypeHints.ReturnTypeHint.UselessAnnotation)
 144 | ERROR | [x] Method \Doctrine\DBAL\Configuration::setDisableTypeComments() has useless @param
     |       |     annotation for parameter $disableTypeComments.
     |       |     (SlevomatCodingStandard.TypeHints.ParameterTypeHint.UselessAnnotation)
------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------

They are reported because the enableStandaloneNullTrueFalseTypeHints parameter defaults to true on PHP 8.2+. I don't think we want to turn it off just to retain the annotations, and we cannot modify the Configuration method signatures because it's not final. Modifying the return type in UnexpectedValue is fine because it is final.

@morozov morozov added the PHP label Apr 4, 2025
@morozov morozov added this to the 4.3.0 milestone Apr 4, 2025
@morozov morozov mentioned this pull request Apr 4, 2025
@morozov morozov force-pushed the php-8.3 branch 2 times, most recently from d042031 to 58e9ada Compare April 4, 2025 16:05
@morozov morozov requested review from greg0ire and derrabus April 4, 2025 16:33
@morozov morozov marked this pull request as ready for review April 4, 2025 16:34
@greg0ire
Copy link
Member

greg0ire commented Apr 4, 2025

This can help making a decision.

Screenshot 2025-04-04 at 23-10-12 PHP Version Stats - doctrine_dbal - Packagist

Apparently 25% of the 4.x userbase still uses 8.2. Only ~3% uses 8.1, so I'd say OK for dropping at least 8.1, but for 8.2, it still feels a bit early.

@morozov
Copy link
Member Author

morozov commented Apr 4, 2025

I think we agreed long ago on a policy that we support the supported PHP versions. The desire to upgrade DBAL will motivate users to upgrade their PHP. They are already using an unsupported PHP versions, and it's apparently not a problem.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Well OK then

@morozov morozov merged commit b59e163 into doctrine:4.3.x Apr 6, 2025
68 checks passed
@morozov morozov deleted the php-8.3 branch April 6, 2025 14:56
@derrabus
Copy link
Member

derrabus commented Apr 7, 2025

Sorry, I'm late to the party here, but this happened during me travelling to conferences. 😓

I too feel that this bump has been too ambitious.

PHP 8.2 is still widely supported by downstream projects like Symfony, TYPO3 and Shopware, just to name a few. Dropping support for it brings those projects into a difficult position, especially if combined with the unreleased changes to our schema API. They're basically forced to use a depreacted API in the next feature release and won't be able to make their projects forward-compatible with DBAL 5. This will ultimately slow down the adoption of DBAL 5.

On top of that, I don't see much benefit for us in bumping to PHP 8.3 because not much has changed in the language itself or the database drivers that would allow us to drop compat code.

I see two possible ways forward:

  • Revert this change entirely and bump the PHP version on the 5.x branch instead. Downstream projects that cannot bump the PHP version with us will be stranded on the last 4.x minor which will receive exended support anyway. I would prefer this route if we intend to release 5.0 within the next 12 months.
  • Move with the ecosystem around us and by lowering the requirement to PHP 8.2. If we don't intend to release DBAL 5 anytime soon, we should still be able to gracefully bump our requirements.

I also think that we should have a more formailzed way to bumping the minimum PHP version. We're having this discussion on each of our repositories and for each of them individually and those discussions feel even more time consuming than the work that we have to invest to support older PHP releases. We've failed to formalize a process for that at our meetup last fall. I will have some time next week and I'll try to prepare something.

@morozov
Copy link
Member Author

morozov commented Apr 7, 2025

I feel strongly against supporting PHP 8.1 which has been out of community support for longer than a year. Restoring the support for PHP 8.2 seems okay given the dependents. Let's establish a more formal agreement on our PHP support policy going forward.

derrabus added a commit that referenced this pull request Apr 9, 2025
|      Q       |   A
|------------- | -----------
| Type         | improvement
| Fixed issues |
#6889 (comment)

#### Summary

This PR allows DBAL 4 to be installed on PHP 8.2 again. See the
discussion on #6889 for details.
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.

3 participants