Up dependencies

[jordisala1991]

Subject

I am targeting this branch, because Symfony 5 support is here.

Changelog

### Removed
- Removed support for Symfony 5.2
- Removed support for SensioFrameworkExtraBundle < 6.1

[OskarStark]

What do we gain by raising the bar?

[jordisala1991]

What do we gain by raising the bar?

IMO not supporting old versions of Symfony (5.2 is EOL). Eventually, when 5.4 comes out we will do the same to only support the LTS from 5.x.

Less dependency versions to maintain, less problems with older versions.

[VincentLanglet]

The psalm error occurs on 3.x too, I'll take a look

[jordisala1991]

The psalm error occurs on 3.x too, I'll take a look

I have fixed some (you might want to pick up to 3.x), the remaining I don't know how to fix them.

[VincentLanglet]

The psalm error occurs on 3.x too, I'll take a look

I have fixed some (you might want to pick up to 3.x), the remaining I don't know how to fix them.

IMHO, it's a bug, I opened an issue vimeo/psalm#6212

[VincentLanglet]

We should first merge #7355 and merge 3.x into master

[OskarStark]

Yes but do we currently have problems? I mean we can raise the bar anytime we struggle with 5.2 🤷🏼‍♂️

[jordisala1991]

Yes but do we currently have problems? I mean we can raise the bar anytime we struggle with 5.2 🤷🏼‍♂️

We do not even know we have problems because we dont have a build for 5.2. And introducing one for testing eol symfony versions is so much time wasted on CI for an unsuported version

[VincentLanglet]

Yes but do we currently have problems? I mean we can raise the bar anytime we struggle with 5.2 🤷🏼‍♂️

We do not even know we have problems because we dont have a build for 5.2. And introducing one for testing eol symfony versions is so much time wasted on CI for an unsuported version

I don't see real issue bumping the Symfony version. Bumping from 5.2 to 5.3 should be straight forward for Symfony user, and it was already our strategy to stop support for EOL version.

I'm more concern bout the SensioFrameworkExtraBundle because it's a conflict. IMHO the purpose of a conflict is to be the more precise possible. Blocking a user because he's using a version 5, when he may not even use the annotation feature is maybe not the best to do.

[jordisala1991]

Yes but do we currently have problems? I mean we can raise the bar anytime we struggle with 5.2 🤷🏼‍♂️

We do not even know we have problems because we dont have a build for 5.2. And introducing one for testing eol symfony versions is so much time wasted on CI for an unsuported version

I don't see real issue bumping the Symfony version. Bumping from 5.2 to 5.3 should be straight forward for Symfony user, and it was already our strategy to stop support for EOL version.

I'm more concern bout the SensioFrameworkExtraBundle because it's a conflict. IMHO the purpose of a conflict is to be the more precise possible. Blocking a user because he's using a version 5, when he may not even use the annotation feature is maybe not the best to do.

We removed a lot of supported versions moving from 3.x to master. I dont know why we should make an exception with this one. Tbh I was tempted to do the same with twig dependency.

[OskarStark]

Fine then

[VincentLanglet]

We removed a lot of supported versions moving from 3.x to master. I dont know why we should make an exception with this one.

Yeah, I remember the last PR: #7023

I won't fight a lot for the keeping the support of this version. I just feel like it's not the semantic of the conflict to be updated.
That's why I saw an exception here, it's not a dependency but a conflict. If you want to remove the 5.6 version from the require-dev, that's debatable but I won't be against. It's not the same than changing the conflict from <5.6 to <6.1.

Tbh I was tempted to do the same with twig dependency.

Twig2 is still supported by twig. We should only remove support for old major without support IMHO.

[VincentLanglet]

@jordisala1991 I fixed the conflict between 3.x and master https://github.com/sonata-project/SonataAdminBundle/pull/7357/files

In order to have green CI I fix the static analysis thanks to your changes.

I think it's more complicated about $httpMethodParameterOverride

Before, the DeleteAction was requiring a DELETE request method ; so the twig was sending a POST request with _method=DELETE. In order to transform this into a DELETE request user should have the http_method_override option to true.
So we had test with DELETE method and also test with POST method with _method=DELETE. And in order to have the second tests working we had to use http_method_override to true in our test (and reset the value at the end of the test).

Now, in master, we support both POST and DELETE. So even if http_method_override is set to false, since it's a POST method, tests are passing. Now we have tests for the three case

• DELETE
• POST
• POST with _method=DELETE with httpMethodOverride TRUE.

Noticing the third test will also works for httpMethodOverride FALSE, it just testing that the request is transformed to DELETE, so it's basically test the Symfony code.

So I think we have more code to remove.

[franmomu]

I'll try this week to see if we can remove the need of SensioFrameworkExtraBundle.

[franmomu]

Thanks @jordisala1991 !