Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
Fixed micro kernel demo#13919
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fixed micro kernel demo#13919
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Let me ping@nicolas-grekas and kindly ask him for a review, because we keep getting more and more pull requests to fix/change the microkernel example and I no longer know if it's right or wrong. Thanks! |
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.
This is correct: I looked at the signature of theadd()
method, the 2nd argument is$path
.
This could target 3.4 btw. |
Thanks@nicolas-grekas! About the issue in the screenshot, how can I fix it please? I don't know where is the problem in the documentation. |
I can't tell precisely without knowing the code. Why don't you extend AbstractController? Or did you forget to enable autoconfiguration? Does adding the container.service_subscriber tag explicitly fix the issue? etc |
It's not my code, it's come from the documentation:https://symfony.com/doc/current/configuration/micro_kernel_trait.html#advanced-example-twig-annotations-and-the-web-debug-toolbar. The tutorial doesn't work and I want to fix it :) |
I created an other PR for this fix. |
wouterj commentedJun 30, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Fyi, I switched the base branch of this PR to 5.1 again. If you switch the base, you also need to rebase your original branch to prevent adding many unrelated commits to this PR. However, we can also switch to 3.4 for you during the merge, so it's fine to keep it like it is now :) Without testing the code, I think we forget to register the controller as a service in this example. You can try adding something like: protectedfunctionconfigureContainer(ContainerBuilder$c,LoaderInterface$loader) {// ...$loader->load(__DIR__.'/../config/services.yaml');// ... } And: # config/services.yamlservices:_defaults:autowire:trueautoconfigure:trueApp\:resource:'../src/*' |
Hello@wouterj, Your code works on my side! I just added this missing part in the PR :) |
javiereguiluz commentedJul 1, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Wouter, if you have some time, could you please merge this PR when it's ready? I don't feel confident enough making changes in this article. Thanks! |
Fine by me. I'll do tomorrow! |
Tomorrow took a bit longer, but I've now fully tested the example code in this article and updated some more bits. Thanks for working on this@Surfoo! Ine1c35a5, I've done a couple more updates:
I've also merged this in 5.1 instead of 3.4. The method signatures in this example requiresymfony/symfony#34873 which was merged in 5.1. Before 5.1, the |
* 5.1: [#13919] Use ContainerConfigurator and fixes some bugs after testing the example code Fixed micro kernel demo
* '5.1' of github.com:symfony/symfony-docs: [#13919] Use ContainerConfigurator and fixes some bugs after testing the example code Fixed micro kernel demo
Uh oh!
There was an error while loading.Please reload this page.
Hello,
Here a fix on the documentation, about the micro kernel demo.
I still have a other bug on the controller:
What's missing to make it work? I'd like to fix it before the merge, can you help me please?