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

Change lowest PHP version to 7 #1255

Merged
merged 2 commits into from
May 17, 2021

Conversation

spinsch
Copy link
Contributor

@spinsch spinsch commented Oct 9, 2020

Description

  • Removes all php version specific fallback fixes
  • Bump min. php version to 7.0.0
  • Replace old magento links

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@github-actions github-actions bot added Component: Authorizenet Relates to Mage_Authorizenet Component: lib/Zend Component: lib/Varien Relates to lib/Varien Component: lib/Mage Relates to lib/Mage Component: lib/* Relates to lib/* labels Oct 9, 2020
@Flyingmana
Copy link
Contributor

PHP 5 is end of life since Dec 2018
https://www.php.net/eol.php

so, Iam not fully opposed to it

@jouriy
Copy link
Contributor

jouriy commented Oct 9, 2020

Please note, that dropping PHP 5.6 support may result in complications for direct upgrades from versions below M1.9.2 running on PHP5.6. So intermediate step with upgrade to M1.9.3+ will be needed first to upgrade PHP before OpenMage upgrade.

@Flyingmana
Copy link
Contributor

There would be the alternative, to only remove this parts from version 20.x on, to make the transition easier.

@spinsch
Copy link
Contributor Author

spinsch commented Oct 12, 2020

Another alternative would be to throw the check completely out of the files and display a hint in the backend instead.

@colinmollenhour
Copy link
Member

Just a comment on the keeping 5.6 idea for 19.x and dropping for 20.x.. This would complicate accepting patches. For example we would have to be careful not to backport 5.x incompatible code from 20.x branch or make sure that patches to 19.x are "upgraded" to PHP 7 for 20.x. Otherwise I like the idea but I'm not sure if it is worth it.

User's clinging to PHP 5.x are sinking lower and lower into that pit.. I'm not really keen on complicating our development cycle for PHP 5 users unless there are really a lot of them and the two-step upgrade path seems like a very reasonable workaround.

Flyingmana
Flyingmana previously approved these changes Oct 20, 2020
@Flyingmana
Copy link
Contributor

for context: current php usage stats for composer. https://blog.packagist.com/php-versions-stats-2021-1-edition/

Copy link
Contributor

@luigifab luigifab left a comment

Choose a reason for hiding this comment

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

Not tested, but I agree, it's time to say good bye to PHP 5.
Also, can I suggest to remove if version_compare in index.php files? :)

@Flyingmana
Copy link
Contributor

Removing of the check I would see as separate topic

@jouriy
Copy link
Contributor

jouriy commented May 16, 2021

Not tested, but I agree, it's time to say good bye to PHP 5.
Also, can I suggest to remove if version_compare in index.php files? :)

de facto, OpenMage cannot run on PHP 5 since v19.4.9 due to 5dee1c0, cf25b8a and 26b9eee, so this check is required now. Migration from older Magento versions require intermediate step with upgrade to 1.9.4.5 first followed by PHP switch to 7.3.

@Flyingmana Flyingmana merged commit 6bdcc28 into OpenMage:1.9.4.x May 17, 2021
@github-actions
Copy link
Contributor

Unit Test Results

1 files  1 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌
6 runs  4 ✔️ 2 💤 0 ❌

Results for commit 6bdcc28.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Authorizenet Relates to Mage_Authorizenet Component: lib/Mage Relates to lib/Mage Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/*
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants