-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix framework instantiation in event-dispatcher #7889
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
Conversation
As the `$resolver` parameter was split up into `$controllerResolver` and `$argumentResolver`, the Framework instantiation call in this code example seems to be outdated and not in line with earlier steps taken in the tutorial.
@iltar you know a lot about this controller/argument resolver thing. Could you please check if this proposal is correct? Thanks! |
@GNi33 which tutorial are you referring to? This page seems to be up-to-date for the latest version (not the master at 4.0.0): https://symfony.com/doc/current/create_framework/http_kernel_controller_resolver.html |
@iltar I'm referring to the page about the Event Dispatcher in the Create Framework tutorial: The pull request changes the code block after the "The last step is the creation..." paragraph, where On the Seperation of concerns page, the instantiation is correct (second code block), as well as on the unit testing page. This is still present in 3.3, as well as in 3.4 |
Looks like the fix is to rename this to |
Yes, you are absolutely right, I missed that. Added it to the pull request. The matcher isn't being initialized as well here. Are these being left out deliberately in this example? |
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.
I don't mind if they are left out if this example is to emphasize initialization or the event listener. In theory it is known what those variables should be. Besides of that initialization (which I have no strong opinion of), it looks good to me!
Thank you @GNi33. |
This PR was squashed before being merged into the 3.2 branch (closes #7889). Discussion ---------- Fix framework instantiation in event-dispatcher As the `$resolver` parameter was split up into `$controllerResolver` and `$argumentResolver`, the Framework instantiation call in this code example seems to be outdated and not in line with earlier steps taken in the tutorial. Commits ------- bdd6791 Fix framework instantiation in event-dispatcher
As the
$resolver
parameter was split up into$controllerResolver
and$argumentResolver
, the Framework instantiation call in this code example seems to be outdated and not in line with earlier steps taken in the tutorial.