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

Bump PHP requirement to 7.3 as of DBAL 2.11.0 #4078

Merged
merged 1 commit into from
Jun 15, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Jun 14, 2020

Q A
Type improvement
BC Break no

Active support for PHP 7.2 by the PHP community ended on November 30, 2019. Since the 2.11.x release series of the DBAL will primary contain deprecations and the forward compatibility layer with the DBAL 3 that will not support PHP 7.3, there's no point in supporting it in 2.11.x either.

@morozov morozov added the PHP label Jun 14, 2020
@morozov morozov added this to the 2.11.0 milestone Jun 14, 2020
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.

The changes look good.
When do we plan to release 2.11.0? It makes sense to me to keep unreleased versions in the green zone of https://www.php.net/supported-versions.php, but I'm wondering if we have a Doctrine-wide policy for this. It would be great if we had this kind of reference somewhere: https://github.com/sonata-project/dev-kit/blob/master/project/CONTRIBUTING.md#dependency-changes , but that would mean getting everyone to agree on something 😅

If we merge this, we should probably also drop 7.2 in doctrine/orm.

@morozov
Copy link
Member Author

morozov commented Jun 14, 2020

When do we plan to release 2.11.0?

The plan is to release it with DBAL 3.0.0 at the time when PHP 8 beta 1 is released. If we can find the specific dates for that, we can update the milestone dates.

I'm wondering if we have a Doctrine-wide policy for this

No, but it's an important discussion. I like how often PHPUnit releases their major versions and I believe they have their policy rules written down somewhere. They define the release cadence and the PHP version support.

If we merge this, we should probably also drop 7.2 in doctrine/orm.

Regardless of that, why does ORM need to support an unsupported PHP version? 😉

@greg0ire
Copy link
Member

I updated the milestones dates based on https://wiki.php.net/todo/php80

Regardless of that, why does ORM need to support an unsupported PHP version? 😉

If I had to guess I would say it has to somehow do with Symfony: the LTS version still provides support for 7.1, and will do so until November 2022, November 2023 if you count security updates, which is the reason why DoctrineBundle also does… and then it raises the question for more packages? Or maybe it has to somehow do with the fact that ORM has a non-dev dependency on symfony/console? I don't know…

For a moment I almost thought we did have a Doctrine-wide policy: https://www.doctrine-project.org/policies/releases.html I feel rick rolled.

@morozov
Copy link
Member Author

morozov commented Jun 14, 2020

I updated the milestones dates based on https://wiki.php.net/todo/php80

Thanks!

This is the PHPUnit release process we could use for inspiration: major release every year, minor release every two months.

@beberlei
Copy link
Member

@greg0ire @morozov btw i am pushing on the deprecations thing mainly to get that ready for 2.11 as well so that you know

@beberlei
Copy link
Member

@morozov its really hard sticking to a schedule for releases when you can't really guarantee that you have time or motivation to work on something. I remember we tried fixed schedules with Doctrine a long time ago, and it quickly evaporated, it can even create pressure that demotivates even more.

@beberlei
Copy link
Member

I think supporting old PHP versions is fine, especially if you don't merge bugs back to old branches, then users might be forced to use the latest version of your library to get bugfixes, but they might have a 3 or 5 year upgrade schedule. In addition PHP maintenance schedule is essentially a "lie", because Linux distributions are willing to guarantee much longer maintenance.

@greg0ire
Copy link
Member

I think supporting old PHP versions is fine, especially if you don't merge bugs back to old branches

An idea I had about that was to allow PRs on old branches that we currently label as unmaintained as long as they are cherry-picks of commits already merged in maintained branch, or if they only apply on said branches. IMO it could be a rare-enough occurrence. I agree with you that it would be preferable to do at least one of those things (supporting old PHP versions or allowing backports)

@morozov
Copy link
Member Author

morozov commented Jun 14, 2020

@morozov its really hard sticking to a schedule for releases when you can't really guarantee that you have time or motivation to work on something. I remember we tried fixed schedules with Doctrine a long time ago, and it quickly evaporated, it can even create pressure that demotivates even more.

Point taken. We can take it easy and just aim at releasing more often w/o any strict schedule. It will at least break the bad habit of trying to do everything before the release and this everything never gets done.

I think supporting old PHP versions is fine

It's fine if we want to. Personally I don't. It's usually the corporate consumers who lag behind the PHP schedule and make money by using the results of our work. I'm fine with supporting a commercial LTS branch but it will have nothing to do with the regular releases.

especially if you don't merge bugs back to old branches

Currently, we do. We expect bugfixes to be targeted at the lowest supported versions.

In addition PHP maintenance schedule is essentially a "lie", because Linux distributions are willing to guarantee much longer maintenance.

It's also done for this corporate part of the ecosystem, not for OSS.

@morozov
Copy link
Member Author

morozov commented Jun 14, 2020

An idea I had about that was to allow PRs on old branches that we currently label as unmaintained as long as they are cherry-picks of commits already merged in maintained branch

As long as we accept commits to the old branch it means that we maintain it.

I agree with you that it would be preferable to do at least one of those things (supporting old PHP versions or allowing backports)

I'd rather be on the “pushing the ecosystem forward” end of the spectrum rather on the LTS one.

@beberlei
Copy link
Member

@morozov i wasn't suggesting we should stay with 7.2 for DBAL 2.11 btw, I do agree more with the sentiment then previously that "why you want your dependencies on the latest version if you cannot upgrade PHP at the same pace". But for many of my libraries I also don't care about supporting 7.0 or 7.1 still for example, it is not usually that big of a burden. "maintaining old versions is fine" was more a general statement :)

A policy of cherry-picking critical fixes backwards is probably better as @greg0ire suggested, as this would also allow users to do most of the work themselves if they know we review, merge + release to older versions for tickets that are "Bug" and released in higher versions.

@morozov
Copy link
Member Author

morozov commented Jun 15, 2020

A policy of cherry-picking critical fixes backwards is probably better […]

I agree that technically it's easier to backport some bugfixes to the not-fully-supported branches than have them as the primary ones where those fixes initially land.

But before we proceed with this discussion, we'll need a definition of the critical issue. We already have a policy of backporting security fixes to the latest non-actively supported version but I don't remember a single case like this in the past 3 years.

As for the non-security related ones, how can an issue be critical if it does exist in a no longer supported release but wasn't noticed there in its entire lifetime? Most likely it's either an edge case or has a reasonable workaround.

@morozov morozov self-assigned this Jun 15, 2020
@morozov morozov merged commit 36c8a78 into doctrine:2.11.x Jun 15, 2020
@morozov morozov deleted the 2.11-php7.3 branch June 15, 2020 15:27
@greg0ire
Copy link
Member

greg0ire commented Jun 15, 2020

IMO we should allow backporting any fix people want to backport, as long as it is a backport. Also, and this is key, we should not do that backporting ourselves as soon as we merge a fix. Instead, we should let people open PRs for the fixes they so badly need to get. IMO, the only fixes we should maybe backport automatically are security fixes.

@morozov
Copy link
Member Author

morozov commented Jun 15, 2020

Also, and this is key, we should not do that backporting ourselves as soon as we merge a fix. Instead, we should let people open PRs for the fixes they so badly need to get.

And who then will maintain the build infrastructure for those builds and people? I.e. Travis may stop supporting the distro that the build is done on, we may stop using Scrutinized or other services in the newer branches, etc.

@greg0ire
Copy link
Member

I'd say that this would also be the burden of contributors. If they can't get a green build because there is no available distro, we just don't merge, they are the ones that have to find a solution. If there is no solution (e.g. no reasonable way to install such an old version of PHP on Travis or whatever), then maintenance just stops "naturally". The philosophy of this workflow is "we will allow you to get unstuck if you do all the work on our terms".

@morozov
Copy link
Member Author

morozov commented Jun 15, 2020

This sounds fair. I think I could live with that.

@kevinpapst kevinpapst mentioned this pull request Dec 17, 2020
10 tasks
rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Mar 7, 2021
Release [2.11.0](https://github.com/doctrine/dbal/milestone/77)

2.11.0
======

- Total issues resolved: **7**
- Total pull requests resolved: **55**
- Total contributors: **8**

Improvement,Prepared Statements,SQL Server,Types,pdo_sqlsrv,sqlsrv
------------------------------------------------------------------

 - [4274: Support ASCII parameter binding](doctrine#4274) thanks to @gjdanis

Documentation
-------------

 - [4271: Add explanation about implicit indexes](doctrine#4271) thanks to @greg0ire

Deprecation,Error Handling
--------------------------

 - [4253: Deprecate DBAL\DBALException in favor of DBAL\Exception](doctrine#4253) thanks to @morozov

CI
--

 - [4251: Setup automatic release workflow](doctrine#4251) thanks to @greg0ire

Deprecation,Schema Managers
---------------------------

 - [4230: Deprecate the functionality of dropping client connections when dropping a database](doctrine#4230) thanks to @morozov

Deprecation,Platforms
---------------------

 - [4229: Deprecate more AbstractPlatform methods](doctrine#4229) thanks to @morozov
 - [4132: Deprecate AbstractPlatform::fixSchemaElementName()](doctrine#4132) thanks to @morozov

Improvement,Test Suite
----------------------

 - [4215: Remove test group configuration leftovers](doctrine#4215) thanks to @morozov
 - [4080: Update PHPUnit to 9.2](doctrine#4080) thanks to @morozov
 - [4079: Forward compatibility with PHPUnit 9.3](doctrine#4079) thanks to @morozov
 - [3923: Removed performance tests](doctrine#3923) thanks to @morozov

Deprecation,Schema
------------------

 - [4213: Deprecate the Synchronizer package](doctrine#4213) thanks to @morozov

Blocker,Improvement,PHP,Test Suite
----------------------------------

 - [4201: Update PHPUnit to 9.3](doctrine#4201) thanks to @morozov

Blocker,PHP,Test Suite
----------------------

 - [4196: The test suite uses the deprecated at() matcher](doctrine#4196) thanks to @morozov

Connections,Deprecation,Documentation
-------------------------------------

 - [4175: Additional deprecation note for PrimaryReplicaConnection::query()](doctrine#4175) thanks to @morozov

Connections,Deprecation,Prepared Statements
-------------------------------------------

 - [4165: Deprecated usage of wrapper components as implementations of driver-level interfaces](doctrine#4165) thanks to @morozov
 - [4020: Deprecated Connection::project(), Statement::errorCode() and errorInfo()](doctrine#4020) thanks to @morozov

Connections,Deprecation
-----------------------

 - [4163: Deprecate duplicate and ambiguous wrapper connection methods](doctrine#4163) thanks to @morozov

Error Handling,Improvement,Types
--------------------------------

 - [4145: Add TypeRegistry constructor](doctrine#4145) thanks to @morozov

Deprecation,Drivers,Improvement,pdo_mysql,pdo_oci,pdo_pgsql,pdo_sqlite,pdo_sqlsrv
---------------------------------------------------------------------------------

 - [4144: Deprecate classes in Driver\PDO* namespaces](doctrine#4144) thanks to @morozov

Connections,Documentation,Improvement
-------------------------------------

 - [4139: Mark connection constructors internal](doctrine#4139) thanks to @morozov

Deprecation,Drivers,Error Handling
----------------------------------

 - [4137: Deprecate driver exception conversion APIs](doctrine#4137) thanks to @morozov
 - [4112: Deprecate DriverException::getErrorCode()](doctrine#4112) thanks to @morozov

Configuration,Connections,Deprecation,Error Handling
----------------------------------------------------

 - [4134: Deprecate some DBALException factory methods](doctrine#4134) thanks to @morozov

Code Style,Documentation
------------------------

 - [4133: Fix more issues introduced by the deprecation of driver classes](doctrine#4133) thanks to @morozov

BC Break,Drivers,Error Handling,pdo_sqlsrv,sqlsrv
-------------------------------------------------

 - [4131: Restore the PortWithoutHost exception parent class](doctrine#4131) thanks to @morozov

Code Style,Improvement,Static Analysis
--------------------------------------

 - [4123: Remove the no longer needed error suppressions](doctrine#4123) thanks to @morozov

Deprecation,Drivers,Error Handling,Improvement
----------------------------------------------

 - [4118: Deprecate ExceptionConverterDriver](doctrine#4118) thanks to @morozov

Bug,Connections,Improvement,Prepared Statements
-----------------------------------------------

 - [4117: Fixes for the recently introduced driver-level deprecations](doctrine#4117) thanks to @morozov

Connections,Deprecation,Platform Detection
------------------------------------------

 - [4114: Deprecate ServerInfoAwareConnection#requiresQueryForServerVersion() as an implementation detail](doctrine#4114) thanks to @morozov

Deprecation,Prepared Statements,SQL Parser,oci8
-----------------------------------------------

 - [4110: Mark non-interface OCI8 driver methods internal](doctrine#4110) thanks to @morozov

Connections,Deprecation,Drivers,Improvement,Prepared Statements
---------------------------------------------------------------

 - [4100: Deprecate inconsistently and ambiguously named driver-level classes](doctrine#4100) thanks to @morozov

Connections,Improvement
-----------------------

 - [4092: Remove Connection::$isConnected](doctrine#4092) thanks to @morozov

Configuration,Connections
-------------------------

 - [4086: Mark Connection::getParams() internal](doctrine#4086) thanks to @morozov

Bug,Drivers,ibm_db2
-------------------

 - [4085: The IBM DB2 driver Exception class must implement the DriverException interface](doctrine#4085) thanks to @morozov

PHP
---

 - [4078: Bump PHP requirement to 7.3 as of DBAL 2.11.0](doctrine#4078) thanks to @morozov

Connections,Databases,Deprecation,Drivers
-----------------------------------------

 - [4068: Deprecate Driver::getDatabase()](doctrine#4068) thanks to @morozov

Deprecation,Improvement,Portability
-----------------------------------

 - [4061: Deprecated platform-specific portability mode constants](doctrine#4061) thanks to @morozov

 - [4054: &doctrine#91;doctrineGH-4052&doctrine#93; Deprecate MasterSlaveConnection and rename to PrimaryReplicaConnection](doctrine#4054) thanks to @beberlei and @albe
 - [4017: Improve help of dbal:run-sql command](doctrine#4017) thanks to @ostrolucky

Code Style,Improvement
----------------------

 - [4050: Update doctrine/coding-standard to 8.0](doctrine#4050) thanks to @morozov

Cache,Deprecation,Improvement,Prepared Statements
-------------------------------------------------

 - [4049: Replace forward-compatible ResultStatement interfaces with Result](doctrine#4049) thanks to @morozov

Cache,Improvement,Prepared Statements
-------------------------------------

 - [4048: Make caching layer not rely on closeCursor()](doctrine#4048) thanks to @morozov

Deprecation,Improvement,Prepared Statements
-------------------------------------------

 - [4037: Introduce Statement::fetchFirstColumn()](doctrine#4037) thanks to @morozov
 - [4019: Deprecated the concept of the fetch mode](doctrine#4019) thanks to @morozov

Bug,Documentation,Improvement,Prepared Statements
-------------------------------------------------

 - [4034: Additional changes based on the discussion in doctrine#4007](doctrine#4034) thanks to @morozov

Connections,Console,Improvement
-------------------------------

 - [3956: allow using multiple connections for CLI commands](doctrine#3956) thanks to @dmaicher

Deprecation,Logging
-------------------

 - [3935: Deprecate EchoSQLLogger](doctrine#3935) thanks to @morozov

Improvement,Packaging
---------------------

 - [3924: Actualize the content of the .gitattributes file](doctrine#3924) thanks to @morozov

Azure,Deprecation,Drivers,Drizzle,MariaDB,Platforms,PostgreSQL,SQL Anywhere,SQL Server,Sharding,pdo_ibm
-------------------------------------------------------------------------------------------------------

 - [3905: Deprecate the usage of the legacy platforms and drivers](doctrine#3905) thanks to @morozov

Deprecation,Query
-----------------

 - [3864: CompositeExpression and()/or() factory methods](doctrine#3864) thanks to @BenMorel
 - [3853: Deprecate calling QueryBuilder methods with an array argument](doctrine#3853) thanks to @BenMorel and @morozov

Deprecation,Tools
-----------------

 - [3861: Deprecated the usage of the Version class](doctrine#3861) thanks to @morozov

Improvement,Query
-----------------

 - [3852: First parameter of ExpressionBuilder::and/or() mandatory](doctrine#3852) thanks to @BenMorel

Deprecation,Improvement,Query
-----------------------------

 - [3851: Rename andX() / orX() methods](doctrine#3851) thanks to @BenMorel
 - [3850: Prepare CompositeExpression for immutability](doctrine#3850) thanks to @BenMorel

# gpg: Signature made Mon Sep 21 01:47:31 2020
# gpg:                using DSA key 1BEDEE0A820BC30D858F9F0C2C3A645671828132
# gpg: Can't check signature: No public key

# Conflicts:
#	README.md
#	lib/Doctrine/DBAL/Driver/AbstractOracleDriver.php
#	lib/Doctrine/DBAL/Driver/OCI8/Driver.php
#	lib/Doctrine/DBAL/Version.php
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants