-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Simplify migration LegacyBridge example #17524
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
Simplify migration LegacyBridge example #17524
Conversation
@dbrumann if you have some time, we'd love to read your opinion about this refactorization. Thanks! |
Let me know if I can provide anything that would expedite or simplify the
review for you. Thanks again for the head start this gave me. First
symfony project for me so every little bit helps. Cheers!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I think the changes are good. I am glad the existing docs helped as a starting point.
1fd1780
to
29505d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. I think this makes it clearer. I have left some more minor suggestions, but nothing critical.
I like this better than the original version, so 👍 from my side.
Thanks for the feedback, everything has been incorporated. Cheers! z |
Friendly ping @derrabus |
No objection from my side. |
54580f3
to
99eecb6
Compare
Great work, thank your for this contribution and congratulations on your first contribution to the Symfony Documentation 👏🎉 |
Rather than return null and then return the prior request, just send the request, and fall back to the LegacyBridge if needed.
I implemented my LegacyBridge following the example given in the docs (thank you for the starting point!) and then realized that the code flow could be simplified. Cheers and regards! jz