Skip to content

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

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

jzohrab
Copy link
Contributor

@jzohrab jzohrab commented Dec 6, 2022

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

@javiereguiluz
Copy link
Member

@dbrumann if you have some time, we'd love to read your opinion about this refactorization. Thanks!

@jzohrab
Copy link
Contributor Author

jzohrab commented Dec 8, 2022 via email

Copy link
Contributor

@dbrumann dbrumann left a 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.

@jzohrab jzohrab force-pushed the simplify_migration_legacy_bridge branch from 1fd1780 to 29505d9 Compare December 9, 2022 02:05
Copy link
Contributor

@dbrumann dbrumann left a 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.

@jzohrab
Copy link
Contributor Author

jzohrab commented Dec 9, 2022

I like this better than the original version, so 👍 from my side.

Thanks for the feedback, everything has been incorporated. Cheers! z

@OskarStark OskarStark changed the title Simplify migration LegacyBridge example. Simplify migration LegacyBridge example Dec 10, 2022
@OskarStark OskarStark requested a review from xabbuh December 10, 2022 09:02
@OskarStark
Copy link
Contributor

Friendly ping @derrabus

@derrabus
Copy link
Member

No objection from my side.

@OskarStark OskarStark force-pushed the simplify_migration_legacy_bridge branch from 54580f3 to 99eecb6 Compare December 12, 2022 11:56
@OskarStark
Copy link
Contributor

OskarStark commented Dec 12, 2022

Great work, thank your for this contribution and congratulations on your first contribution to the Symfony Documentation 👏🎉

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.

6 participants