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

Make php-8.1 minimum requirement #4124

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

Conversation

sreichel
Copy link
Contributor

@sreichel sreichel commented Jul 28, 2024

We removed 7.3 support in 03/2003 ... now - plus one year later - its time to make the next step.

PHP8 offers a lot of good features, i'd like to use when writing extension but i am limited due to php7 support.

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

According to packagist.org stats only 20% still use php7.4 with latest releases.

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


Note 1: If you have modules/extensions that dont work with php8, drop me a note! This should be no reason to stay on an old php version!

Note 2: OpenMage will still proberly work with older versions too - as long we not introduce backwards-incomaptible changes.

Note3 : with https://github.com/rectorphp/rector you have a tool to refactor your code for php8.

* Rector: CQ - UnusedForeachValueToArrayKeysRector

See Rector\CodeQuality\Rector\Foreach_\UnusedForeachValueToArrayKeysRector

* fixes + phpstan

See fix at rector: rectorphp/rector-src#6164
- updated workflows
- phpstan baseline (will be fixed later)
@sreichel sreichel changed the title Make php-8.,0 minimum requirement Make php-8.0 minimum requirement Jul 28, 2024
@sreichel sreichel marked this pull request as draft July 28, 2024 14:58
composer.json Outdated Show resolved Hide resolved
sreichel and others added 3 commits August 7, 2024 06:09
Co-authored-by: Ng Kiat Siong <[email protected]>
# Conflicts:
#	composer.lock
#	phpstan.dist.baseline.neon
@github-actions github-actions bot added Component: lib/Varien Relates to lib/Varien Component: Media Relates to Mage_Media Component: Install Relates to Mage_Install Component: lib/* Relates to lib/* labels Sep 4, 2024
@sreichel sreichel changed the title Make php-8.0 minimum requirement Make php-8.1 minimum requirement Sep 4, 2024
@sreichel sreichel marked this pull request as ready for review September 4, 2024 23:20
addison74
addison74 previously approved these changes Sep 5, 2024
@sreichel sreichel marked this pull request as draft September 5, 2024 23:05
@colinmollenhour
Copy link
Member

colinmollenhour commented Sep 9, 2024

Agreed with the changes, but I believe this should be in "next" so that it is in the next major version update.

MAJOR version when you make incompatible API changes

  • ...
  • dependencies updated in major ways (e.g. dropping support of PHP 7.4 or MySQL 5.6)

All new PRs are based on either "main" branch for MINOR and PATCH updates or "next" branch for MAJOR updates

Since v21 was not released yet this could still make it into v21.

EDIT: Just saw there is already #3920

@sreichel
Copy link
Contributor Author

sreichel commented Sep 9, 2024

@colinmollenhour its on draft, b/c we should think about to raise minimum version for main branch too.

# Conflicts:
#	.github/workflows/phpstan.yml
#	.github/workflows/sonar.yml
#	.github/workflows/syntax-php.yml
#	app/code/core/Mage/Media/Model/File/Image.php
#	composer.json
#	composer.lock
#	phpstan.dist.baseline.neon
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.

4 participants