phoenix
phoenix copied to clipboard
phoenixframework
Drop doctrine/dbal 2.* support
Hi all,
Is there a reason to support unmaintained doctrine/dbal 2.* ?
Among many other things (in the 3.* version) MySql was renamed to MySQL see there, it is still MySql in the current codebase , e.g. here - that fails some code analyzers
According to https://www.doctrine-project.org/2022/01/22/sunsetting-dbal-2.html, the EOL date for DBAL 2.x is 6 months after the release date of ORM 2.10 (supporting DBAL 3), which was on April 3 2022. this means that DBAL 2 is not EOL yet. It will reach EOL only in October.
Right, so they plan to address security issues after that for at most a year, does that mean you will keep supporting it as well?
Is it worth creating a new branch without 2.* support? As at the moment it is not possible to even backport MySQL renaming to the 2.* branch
https://github.com/doctrine/orm/releases/tag/2.10.0 was actually released on October 3rd 2021, the phrasing has mislead you @stof, I'm afraid :wink:
We should IMO drop DBAL 2 support from the 2.8.x branch (which does not exist yet but I can fix that)
Yes we can drop 2.x if it means we can support some 3.x feature more easily. But doesn't mean to just increase composer.json constraint, but also drop all the BC layers.
I'm happy to rename MySql to MySQL once 2.* is dropped. Or it can be done now - the only difference is when the errors are generated - with dbal 3 or dbal 2 installed. At the moment it happens with dbal 3, which I would expect is the majority of the cases.
@ostrolucky do we need a new major to properly drop every BC layer for DBAL 2? I guess we would have to remove some classes. I mean they are useless/broken anyway with DBAL 3 but not sure if we can remove them in a minor release?
Like
https://github.com/doctrine/DoctrineBundle/blob/2.7.x/Command/Proxy/ImportDoctrineCommand.php https://github.com/doctrine/DoctrineBundle/blob/2.7.x/Command/Proxy/DoctrineCommandHelper.php
and probably some others.
Depends what we need to change. These commands can be removed IMHO, but what else is there? Eg. changing config options that would be a bigger BC break.
@ostrolucky here are (minimal) changes related to commands: https://github.com/doctrine/DoctrineBundle/compare/2.8.x...dmaicher:DoctrineBundle:drop_dbal_2?expand=1
We would also need to remove the shards config option: https://github.com/doctrine/DoctrineBundle/blob/2.7.x/DependencyInjection/Configuration.php#L236
Not sure if doing this on 2.x is a good idea? :confused:
Make it a no-op instead?
@greg0ire hm but if someone was configuring shards and then suddenly this just silently stops working this is a bit unexpected, or? I mean we could keep the option and just throw a meaningful exception if someone is configuring it :thinking:
@ostrolucky @greg0ire here would be changes that keep the sharding related config options but throw exceptions in case they are used: https://github.com/doctrine/DoctrineBundle/commit/3969aae73a8a5415f46e0ca0519ba7b6e716285d
Do you think this is something we could release on 2.x?
I don't see how throwing exception is better than removing those config options. If throwing in 2.x is ok for you then I would just remove those options in 2.x
Yeah I guess the only difference would be a slightly better error message. But probably not worth keeping the options then indeed.
I'm fine with removing them completely on 2.x. Currently people would be getting errors already if they try to use Sharding with DBAL 3. So we don't break anything technically.