Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
wouterj merged 1 commit intosymfony:5.1fromSurfoo:5.1
Jul 7, 2020
Merged

Fixed micro kernel demo#13919

wouterj merged 1 commit intosymfony:5.1fromSurfoo:5.1
Jul 7, 2020

Conversation

Surfoo
Copy link
Contributor

@SurfooSurfoo commentedJun 29, 2020
edited
Loading

Hello,

Here a fix on the documentation, about the micro kernel demo.

I still have a other bug on the controller:

mk

What's missing to make it work? I'd like to fix it before the merge, can you help me please?

@SurfooSurfoo changed the titleFixed micro kernel traitFixed micro kernel demoJun 29, 2020
@javiereguiluz
Copy link
Member

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!

Surfoo reacted with thumbs up emoji

Copy link
Member

@nicolas-grekasnicolas-grekas left a 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.

@nicolas-grekas
Copy link
Member

This could target 3.4 btw.

@SurfooSurfoo changed the base branch from5.1 to3.4June 30, 2020 18:03
@Surfoo
Copy link
ContributorAuthor

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.

@nicolas-grekas
Copy link
Member

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

@Surfoo
Copy link
ContributorAuthor

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 :)

@wouterjwouterj changed the base branch from3.4 to5.1June 30, 2020 18:53
@wouterjwouterj added this to the3.4 milestoneJun 30, 2020
@SurfooSurfoo mentioned this pull requestJun 30, 2020
@Surfoo
Copy link
ContributorAuthor

I created an other PR for this fix.

@wouterj
Copy link
Member

wouterj commentedJun 30, 2020
edited
Loading

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/*'

@Surfoo
Copy link
ContributorAuthor

Hello@wouterj,
Thanks for the tips, I didn't know.

Your code works on my side! I just added this missing part in the PR :)

@javiereguiluz
Copy link
Member

javiereguiluz commentedJul 1, 2020
edited
Loading

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!

@wouterj
Copy link
Member

Fine by me. I'll do tomorrow!

wouterj added a commit that referenced this pull requestJul 7, 2020
@wouterjwouterj merged commit6d1ec6b intosymfony:5.1Jul 7, 2020
@wouterj
Copy link
Member

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:

  • Use the new configurator signature forconfigureContainer() as well. This removes the need for the Yaml file - it can be done in PHP directly.
  • Added some PHP typing
  • Fixed some paths in the imports

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, theRouteCollection andContainerBuilder classes where used in the microkernel. Those examples are still working fine (problems started when we only updated the type hints, but not the method calls).

wouterj added a commit that referenced this pull requestJul 7, 2020
* 5.1:  [#13919] Use ContainerConfigurator and fixes some bugs after testing the example code  Fixed micro kernel demo
javiereguiluz added a commit that referenced this pull requestJul 8, 2020
* '5.1' of github.com:symfony/symfony-docs:  [#13919] Use ContainerConfigurator and fixes some bugs after testing the example code  Fixed micro kernel demo
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.

5 participants
@Surfoo@javiereguiluz@nicolas-grekas@wouterj@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp