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

Fix PHP 8.4 deprecation warnings in 6.x #761

Open
wants to merge 3 commits into
base: 6.0.x
Choose a base branch
from

Conversation

W0rma
Copy link
Contributor

@W0rma W0rma commented Nov 5, 2024

From #726 (reply in thread):

I think we could do the same there, tag 6.0 then work on 6.1 bumping PHP and doing all the changes

This PR uses a similar strategy as in #731 but for 6.x:

  • bump minimum PHP version to 7.1
  • fix PHP deprecation warnings

@DannyvdSluijs
Copy link
Collaborator

Hi @W0rma

Thanks for the contribution. I was curious why you're targeting the 6.0.x branch? As that branch is for patch release for version6.x and we should not drop support of dependency versions in a patch release.

The changes (besides the one in .php-cs-fixer.dist.php) are already part of the master branch. So all it needs is work towards a release of a new mayor version. The discussion you linked has a todo list on what (ideally) needs to be done.

Also if you think the changes in .php-cs-fixer.dist.php are needed can you create a PR adding those changes to the master branch?

@W0rma
Copy link
Contributor Author

W0rma commented Nov 7, 2024

So all it needs is work towards a release of a new mayor version.

@DannyvdSluijs Will the next release be a major (7.0) or a minor (6.1) version?

I wasn't sure if master already contains BC breaks and my aim was to fix the deprecation warnings for the next 6.x minor version.

@DannyvdSluijs
Copy link
Collaborator

@W0rma Im currently to only developer pushing this project forward. Im included to work towards a 7.0.0 release.
Best would be to have a fix for master and port what ever is needed to the 6.x branch.

@W0rma
Copy link
Contributor Author

W0rma commented Nov 21, 2024

I'm happy to assist as far as I can. Could you create a 6.x branch so I can prepare a PR which ports the fixes for the PHP 8.4 deprecation warnings?

@DannyvdSluijs
Copy link
Collaborator

Ideally we get @Seldaek to rename the current 6.0.x branch to 6.x (I don't have permissions to do so) so we have a proper branch for version 6. And all the work done in master is going to be 7.0 (which I think releasing sooner than later would be the best option).

This would also help #776 move forward for other people.

@erayd
Copy link
Contributor

erayd commented Jan 28, 2025

@DannyvdSluijs For what it's worth, in my opinion you should be set as a project owner. You're the main person driving this library forward, and frankly the only trusted party who has time. And you're clearly in it for the long haul; you've been working on this for quite some time now. It's been wonderful to see some progress 😁

@Seldaek What do you think? I reckon giving @DannyvdSluijs those permissions seems reasonable.

@Seldaek
Copy link
Contributor

Seldaek commented Jan 28, 2025

Yeah that makes total sense. I've granted you owner access on the organization @DannyvdSluijs :)

@DannyvdSluijs
Copy link
Collaborator

@W0rma if you could revert the changes is composer.json we could merge this one. After which I'll rename the targetted branch (doing so before would close this PR) and make a new 6.x release

@W0rma
Copy link
Contributor Author

W0rma commented Jan 28, 2025

if you could revert the changes is composer.json we could merge this one.

@DannyvdSluijs Sure, I can do that.

Just to make sure that there is no misunderstanding: The version bump is necessary because nullable types require at least PHP 7.1 (see https://www.php.net/manual/en/migration71.new-features.php#migration71.new-features.nullable-types).

Do you plan to add the change again after this PR is merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants