Allow Symfony 7, dropped support for 4.4 and bumped minimal version to 5.4

[yann-eugone]

Hello, first of all, thank you for your contribution Can you explain why you wish we drop support for older php/symfony versions ? Is there something that is not working in your cases ?

[evertharmeling]

Hi Yann,

Thanks for your response! Main thing is that Symfony 4.4 isn't maintained anymore, security fixes end this month, also see the overview of all supported versions.

Next to that the adjustment in src/Messenger/DumpSitemapMessageHandler.php would be much more complicated to fix as the AsMessageHandler attribute is introduced in Symfony 5.4. Supporting both the interface MessageHandlerInterface (not being available in Symfony >=7.0) and the attribute AsMessageHandler (not being available in Symfony <5.3) would require some (unwanted) hacks. Dropping support for 4.4 solves this problem.

So all in all I think this justifies dropping the support for 4.4. Users who still use that version could easily keep using version 3.3.1 of this bundle.

[yann-eugone]

I agree Still, this will require a new major version, so marked this PR as bc break

[evertharmeling]

It would not cause a BC break, and wouldn't necessarily require a new major version, as users who are on Symfony 4.4 won't be able to install a newer version of the bundle as they do not meet the requirements (symfony 5.4 and up).

But you could still release a new version if you want. Cheers!

[yann-eugone]

I see your point But I still consider dropping support for Symfony versions to be done in a major release Removing interfaces from classes might also be considered as BC, because you don't know what tools developers might have created on this

Still very appreciated, and will definitely merge it whenever the CI will be green

[evertharmeling]

Sure!

I dropped the usage of symfony/framework-extra-bundle as it is deprecated since Symfony 6.2 and incompatible with Symfony >=7.0. AFAIK it was only used for the Route annotations, as PHP attributes are the new best practices but only supported from Symfony >=6.4, I refactored lower versions to make use of Yaml routes.

This should fix the CI build hopefully.

[evertharmeling]

Could you approve the workflow Yann, so we at least see if the CI is all fixed now?

[yann-eugone]

I was sure I approved it... Sorry

[evertharmeling]

No problem! I've fixed some tests and now need your approval again :). Hopefully the suite passes for all versions now.

[evertharmeling]

Hmm they changed the return type of KernelTestCase::getContainer() from ContainerInterface to Container (see). Used in SitemapTestCase.

Not really sure how we may bypass this easily...

EDIT: maybe the same solution as done with Kernel :), let me see

Alright looks like this could work, could you approve the workflow once again :)

[evertharmeling]

Ah sorry I seem to have messed up the contents of the /special/annotations.yaml file, fixed it. One last time approving the workflow 🤞

[dkarlovi]

@yann-eugone

I still consider dropping support for Symfony versions to be done in a major release

This is typically not the case, dropping support for specific dependencies is not a breaking change because the users of the dropped dependency will not get the new package offered, no?

when we drop support for an older version of PHP, composer will not consider the new version if the PHP version requirement is no longer fulfilled. Thus, you won't end up with a fatal error due to a wrong method signature, you just won't get the new version.

You don't need to release a major version just to stop supporting an old dep version.

[yann-eugone]

@dkarlovi sure, I've understand this point of view I believe everyone is free to deal with this kind of problem the way they want, I understand why Doctrine is doing it like it, and that's OK But as you can see in the first paragraph of the link you provided

Since many people are encountering issues with these updates ...

And here is the issue : people complains for almost anything As a maintainer of package, I want to be clear what the contract are when using the bundle And to me, it's very clear that :

• dropping version of a package
• dropping compatibility with a package
• dropping compatibility with a feature of a package

Remains a BC

[yann-eugone]

hmmm ok I've missed something We already have a 4.x branch, and these devs here should have been done on that branch do you want to rebase your branch @evertharmeling ?

[evertharmeling]

@yann-eugone done!

No problem, glad I could be of help! Thanks for your help and patience!

[Valmonzo]

Hello, what is missing to merge this PR ? Any helps needed ?

[shmshd]

@yann-eugone, just wondering if there's any particular reason for the delay in merging this PR?

[yann-eugone]

I was unavailable for some holidays :christmas_tree:

Unfortunately, it seems that the rebase of that branch picked some commits we don't want. These are commits added to the 3.x branch for the upgrade path to 4.x.

I understand this is partially my fault, because I didn't told you this from the beginning you targeted the wrong branch, still, these commits must be dropped in order for that PR to be merged.

[evertharmeling]

Sorry missed the commits popped in, I removed them and it should be fixed now! @yann-eugone could you approve the workflow once again?

[yann-eugone]

Thank you for your help, very much appreciated

[evertharmeling]

No problem, glad to be of help! Thanks for the feedback and assistance!

[CoalaJoe]

Thanks for the work guys! Is there a release already planned containing these changes?

[yann-eugone]

Not yet, the next major version will take some time to be tested

You can help by requiring the dev version on your projects

composer require presta/sitemap-bundle:4.x-dev@dev

And provide feedback by opening issues if something fails

[Valmonzo]

Thx for your works guys !