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

PHP 8 rector recommended changes #21

Merged
merged 4 commits into from
Feb 10, 2024
Merged

Conversation

franzholz
Copy link
Contributor

@bmack
Copy link
Member

bmack commented Mar 30, 2023

This can only be merged if we only support PHP 8+ (which I guess is only possible if we drop v10+v11 support). to be honest, for most of the changes, I don't see a benefit for the code base.

@franzholz
Copy link
Contributor Author

It is time to require PHP 8 as a minimum.

@ohader
Copy link
Contributor

ohader commented Feb 7, 2024

Actually it's time to get rid of $GLOBALS['TYPO3_DB'] and migrate to use Doctrine DBAL (which was introduced with TYPO3 v8 in 2016: https://docs.typo3.org/m/typo3/reference-coreapi/11.5/en-us/ApiOverview/Database/Migration/Index.html

@franzholz
Copy link
Contributor Author

More years are needed to get all the extensions to Doctrine DBAL. extensions.typo3.org shows still more than 8000 downloads in February 2024.

@bmack
Copy link
Member

bmack commented Feb 9, 2024

@franzholz still, can you update the branch to only allow PHP 8 in composer.json?

@franzholz
Copy link
Contributor Author

I have updated nowe the php8-rector branch to require PHP 8 and TYPO3 12.

Copy link
Contributor

@sgrossberndt sgrossberndt left a comment

Choose a reason for hiding this comment

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

I don't endorse this change as removing the backwards compatibility on a legacy extension meant to ease migrating to QueryBuilder really doesn't seem sane. On the other hand supporting TYPO3 9.4-12 in this extension would make it less maintainable.

ext_emconf.php Outdated
@@ -13,7 +13,8 @@
'version' => '1.1.5',
'constraints' => [
'depends' => [
'typo3' => '9.0.0-11.5.99',
'php' => '8.0.0-8.4.99',
'typo3' => '12.0.0-12.4.99',
'backend' => '9.0.0-11.5.99',
Copy link
Contributor

Choose a reason for hiding this comment

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

Conflicting outdated version requirement

ext_emconf.php Show resolved Hide resolved
@sgrossberndt
Copy link
Contributor

Also this change is based on an outdated state of the extension and cannot be merged

@sgrossberndt
Copy link
Contributor

Franz, I understand you work on this extension in order to gain TYPO3 v12 support for EXT:tt_products, but as even your extension in the current state supports TYPO3 v10+ - all of your supported TYPO3 versions do support QueryBuilder and instead of dedicating work into making typo3db_legacy on newer TYPO3 versions you should focus on making tt_products work with QueryBuilder instead.

@franzholz
Copy link
Contributor Author

Yes, of course I should invest many hours in dozens of extensions. Obviously you have not noticed many other extensions which I maintain. However it just takes to much time to get them ajll there. This is not the only thing which must be done one day. There are so many other reprogramming issues and bug fixings to do.

@franzholz
Copy link
Contributor Author

franzholz commented Feb 9, 2024

I have made a new patch for rector under PHP 8.

@franzholz franzholz reopened this Feb 10, 2024
@bmack bmack merged commit ef80214 into FriendsOfTYPO3:main Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants