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

task: add support for php 8.2 #227

Merged
merged 1 commit into from
Oct 10, 2023
Merged

task: add support for php 8.2 #227

merged 1 commit into from
Oct 10, 2023

Conversation

xaevik
Copy link
Contributor

@xaevik xaevik commented Sep 6, 2023

Description

Adds support for PHP 8.2 for Magento >= 2.4.6 installations

@shochdoerfer
Copy link
Member

@xaevik awesome, thanks for helping out!

@shochdoerfer
Copy link
Member

@xaevik the CI build fails because we sticked to Composer 1 for a long time (to be able to install the module also on older Magento instances). Now that hurts us ;(

I think the best would be to upgrade to Composer 2 and not care about older Magento versions anymore. Since we run the builds for Magento 2.4.x, I think all those versions should be compatible with Composer 2. Or what do you think?

@xaevik
Copy link
Contributor Author

xaevik commented Sep 7, 2023

@shochdoerfer if you're only targeting Magento >= 2.4.x at this point, I would safely say bumping the composer version to >= 2.2 in the CI pipeline would be wise, I can update the PR if you'd like to include those changes.

As far as I remember, stock Magento 1.x never supported composer as a first party concern, but I know the OpenMage (which is an M 1.x fork) project does.

@xaevik
Copy link
Contributor Author

xaevik commented Sep 7, 2023

@shochdoerfer I updated ci.yml to reflect the necessary changes:

  • Set minimum composer version to 2
  • Remove EOL releases of Magento
  • Set minimum PHP version to 8.1

This should allow the pipeline to run properly.

@xaevik
Copy link
Contributor Author

xaevik commented Sep 7, 2023

Looks like a CodeSniffer dependency is missing, I'll address that shortly.

@xaevik
Copy link
Contributor Author

xaevik commented Sep 7, 2023

Ok, looks like some more work is required here. Unfortunately captainhook is giving me some difficulties on my local but I'll set some time aside to look at this deeper tomorrow.

@shochdoerfer
Copy link
Member

No worries. You can skip the git hook if that is easier for you ;) As long as we get the build green, I am fine.

@xaevik
Copy link
Contributor Author

xaevik commented Sep 7, 2023

@shochdoerfer it looks as though phpstan.neon had an outdated ignoreErrors value. I removed it, ran tests locally and squashed all the commits. CI should (hopefully) return green this time.

@shochdoerfer
Copy link
Member

Fingers crossed ;)

@shochdoerfer
Copy link
Member

@xaevik looks good! Thx for the help. I know the version upgrades can be a bit annoying. Very much appreciate your support.

Will try to release a new package in the next few days. Just need to find the time for it.

@shochdoerfer
Copy link
Member

@xaevik just wondering: Should we also remove the older PHP versions and Magento module versions from composer.json? I mean older versions of the module still work with those older versions and ideally the CI pipeline covers all versions we actively support.

@xaevik
Copy link
Contributor Author

xaevik commented Sep 8, 2023

@shochdoerfer my general rule of thumb is to only support what is not considered end-of-life, in this case Magento >= 2.4.4 and PHP >= 8.1 (I need to add Magento 2.4.6 to the CI as I forgot).

I'll make some further refinements to composer.json as well to bring everything up to snuff.

Signed-off-by: Alan Brault <alan.brault@visus.io>
@xaevik
Copy link
Contributor Author

xaevik commented Sep 8, 2023

@shochdoerfer all right I've made the following changes:

  • Updated composer.json dependencies to only support Magento >= 2.4.4 (see life cycle for details).
  • Bumped the version from 5.1.0 to 6.0.0 as this is considered a breaking change.
  • Updated ci.yml to test against 2.4.4, 2.4.5, and 2.4.6 (tested locally with nektos/act)

Technically the direct PHP requirement in composer.json is not necessary as the direct dependency on magento/framework already takes care of this, but if you ever want to employ outside tools such as Mend Renovate or Dependabot it helps.

@shochdoerfer shochdoerfer added this to the 5.2.0 milestone Oct 10, 2023
@shochdoerfer shochdoerfer merged commit 4e2d8e9 into bitExpert:master Oct 10, 2023
7 checks passed
@xaevik xaevik deleted the php82-support branch October 10, 2023 16:01
@shochdoerfer
Copy link
Member

@xaevik thanks for helping out. And once again sorry for letting the PR hang for so long. Will release a version hopefully during this week.

@xaevik
Copy link
Contributor Author

xaevik commented Oct 10, 2023

@shochdoerfer hey no worries, it's out now and now everyone can benefit from the change.

@barryvdh
Copy link

Can you tag an update for this? Thanks!

@shochdoerfer
Copy link
Member

May bad, wanted to tag a release last week during my holiday, but somehow, things got out of my hand. Give me a day or two to get this done.

@shochdoerfer
Copy link
Member

@barryvdh
Copy link

Thanks!

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.

PHP 8.2 support
3 participants