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

Symfony 5.x support #31

Merged
merged 5 commits into from
Sep 6, 2020
Merged

Symfony 5.x support #31

merged 5 commits into from
Sep 6, 2020

Conversation

kiettran
Copy link
Contributor

@kiettran kiettran commented Jul 21, 2020

This pull request adds full symfony 5 support. Do note, this pull request does not have symfony 3.4 or symfony 4.x support anymore.

#23

@xvilo
Copy link

xvilo commented Aug 15, 2020

@kiettran any idea on when you will finish this?

@kiettran
Copy link
Contributor Author

kiettran commented Aug 15, 2020

It’s done except two tests are failing because the classes are final and cannot be mocked. The pull request does work within a symfony 5 project. I will have a look on the tests this week.

It's done.

@niels-nijens
Copy link
Member

Thanks for the effort (again).

In its current form I'm unable approve this. Symfony 4.4 is supported until the end of 2022 and I'm unwilling to drop support for it when Symfony does not.

Though, I am willing to drop support for Symfony 3.4 to make the upgrade path easier.

A lot is going on in this pull request and it's not just Symfony 5.x support. It should only contain just that.

Copy link
Member

@niels-nijens niels-nijens left a comment

Choose a reason for hiding this comment

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

Finally had some time to review in more detail. I still think it is possible to support all three currently maintained Symfony versions.

.gitignore Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@@ -71,7 +71,7 @@ public function __construct(
/**
* Validates the body of a request to an OpenAPI specification route. Throws an exception when validation failed.
*/
public function validateRequestBody(GetResponseEvent $event): void
public function validateRequestBody(RequestEvent $event): void
Copy link
Member

Choose a reason for hiding this comment

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

By changing to the following, it's made compatible with both Symfony 3.4 and 4.4/5.1 (including the above suggestions):

Suggested change
public function validateRequestBody(RequestEvent $event): void
public function validateRequestBody($event): void

@niels-nijens
Copy link
Member

I noticed after my review that the change of type hints in the RouteLoader might be the only backwards compatibility problem.

For this I tried the following changes:
master...niels-nijens:route-loader-type-hints

Although it's not the prettiest code, it works like a charm. 😄

When you rebase to master and possibly fix the conflicts, I can PR it to your branch.

If you need any help getting this ready. Let me know!

@niels-nijens niels-nijens added this to the 1.0.0 milestone Sep 1, 2020
@niels-nijens niels-nijens linked an issue Sep 1, 2020 that may be closed by this pull request
kiettran and others added 2 commits September 2, 2020 17:19
Co-authored-by: Niels Nijens <nijens.niels@gmail.com>
Co-authored-by: Niels Nijens <nijens.niels@gmail.com>
@niels-nijens niels-nijens merged commit 3eb9747 into nijens:master Sep 6, 2020
@niels-nijens
Copy link
Member

Thanks!

@kiettran kiettran deleted the symfony5 branch September 23, 2020 13:38
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.

Symfony 5.0 compatibility
3 participants