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.

Sign up for GitHub

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

Jump to bottom

Replace ZF1Future with ZF1S #4118

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

Replace ZF1Future with ZF1S #4118

wants to merge 18 commits into from

Conversation

sreichel
Copy link
Contributor

... later ...

ToDo:

  • diff
  • apply OM patches

@sreichel
Rector: CQ - UnusedForeachValueToArrayKeysRector (#1)
3d7eaf6 
* Rector: CQ - UnusedForeachValueToArrayKeysRector

See Rector\CodeQuality\Rector\Foreach_\UnusedForeachValueToArrayKeysRector

* fixes + phpstan

See fix at rector: rectorphp/rector-src#6164
@sreichel
Merge branch 'OpenMage:main' into main
53ee2f8
@sreichel
Merge branch 'OpenMage:main' into main
89cc37c
@sreichel
Replace ZF1Future with ZF1S
bf10e20
@github-actions github-actions bot added Component: PayPal Relates to Mage_Paypal Component: Core Relates to Mage_Core Component: Catalog Relates to Mage_Catalog Component: Reports Relates to Mage_Reports Component: Checkout Relates to Mage_Checkout Component: lib/Varien Relates to lib/Varien Component: Sales Relates to Mage_Sales Component: Customer Relates to Mage_Customer Component: Eav Relates to Mage_Eav Component: Adminhtml Relates to Mage_Adminhtml Component: Page Relates to Mage_Page Component: Tag Relates to Mage_Tag Component: Weee Relates to Mage_Weee Component: Shipping Relates to Mage_Shipping Component: Rule Relates to Mage_Rule Component: Payment Relates to Mage_Payment Component: Index Relates to Mage_Index Component: Api2 Relates to Mage_Api2 Component: ImportExport Relates to Mage_ImportExport Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches Component: Dataflow Relates to Mage_Dataflow Component: Install Relates to Mage_Install Component: Paygate Relates to Mage_Paygate Component: lib/* Relates to lib/* composer Relates to composer.json phpstan labels Jul 25, 2024
@github-actions github-actions bot removed Component: Paygate Relates to Mage_Paygate Component: lib/* Relates to lib/* phpstan labels Jul 25, 2024
@sreichel sreichel marked this pull request as draft July 25, 2024 10:31
@sreichel sreichel changed the title Draft: Replace ZF1Future with ZF1S Replace ZF1Future with ZF1S Jul 25, 2024
@ADDISON74
Copy link
Contributor

What are the advantages changing ZF1- Future with ZF1S?

@sreichel
Copy link
Contributor Author

sreichel commented Jul 25, 2024 •
edited
Loading

IMHO ... we dont have to add the complete Zend framework ... only the parts/packages we need.

And ... i'd like fix #3462

#4117 did not solve it. As @elidrissidev and my tests confirmed ZF1S works for some reason. I'm to lazy to search for ... better spent a few hours here. I already did it with ZF1Future so it will not take that long again ...

@sreichel
Copy link
Contributor Author

Green for now ...

  • diff for php8 changes
  • apply OM patches
  • add PRs to ZF1S (to replace that patches)

... done :)

@elidrissidev
Copy link
Member

There are some important changes that zf1s doesn't include by default such as the flag to prevent PDO from casting numeric types in PHP 8:

https://github.com/Shardj/zf1-future/blob/94d85abd45bdcb806b24a9adbe7c9eba65d54ba3/library/Zend/Db/Adapter/Pdo/Mysql.php#L113-L119

@sreichel
Copy link
Contributor Author

sreichel commented Jul 25, 2024 •
edited
Loading

Thanks. If follow ZF1Future too and make a diff later.

(btw ... i'd like to have casting to int/not casting to string, .... drop php 7!!! :P)

@elidrissidev
Copy link
Member

(btw ... i'd like to have casting to int/not casting to string, .... drop php 7!!! :P)

I prefer that too, but there will be a lot of hard to find bugs, so I suggest leaving it as is for a separate PR in the future.

@ADDISON74
Copy link
Contributor

ADDISON74 commented Jul 25, 2024 •
edited
Loading

Don't fix something that works :)

I would not drop support for PHP 7.4, I am using servers running Debian 11 and it is entering the LTS program until June 2026 and which I will not upgrade any time soon.

Debian LTS: https://wiki.debian.org/LTS
Debian 11: https://www.debian.org/News/2021/20210814 => PHP 7.4 - End of August 2026

@sreichel
Copy link
Contributor Author

Don't fix something that works :)

Let us dream ... :P

O/c thats something that will not happen in near future.

@empiricompany
Copy link
Contributor

+1 to drop php 7 support
https://www.php.net/eol.php

@sreichel
Copy link
Contributor Author

On hold.

There are a lot of fixes in ZF1Future, but not in ZF1S.

I already started to prepare PRs, but this will take some time to make it good.

I like that mono-repo idea, but atm it seems to be a step backwards.

So we have to search for the not working autoloading from app/local|community|core ...

@sreichel
Copy link
Contributor Author

sreichel commented Jul 27, 2024 •
edited
Loading

Debian LTS: https://wiki.debian.org/LTS
Debian 11: https://www.debian.org/News/2021/20210814 => PHP 7.4 - End of August 2026

We have droped php7.3 support in 03/2023.

Imho it would be great to could make use of fancy php8 stuff.

https://accesto.com/blog/php-performance-improvement-features/

Its arround 20% that still use latest releases with php7.4.

https://packagist.org/packages/openmage/magento-lts/php-stats#20.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Component: PayPal Relates to Mage_Paypal composer Relates to composer.json phpstan
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sreichel @ADDISON74 @elidrissidev @empiricompany