Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use ContainerAwareTrait #5884

Closed
ghost opened this issue Nov 12, 2015 · 9 comments
Closed

Use ContainerAwareTrait #5884

ghost opened this issue Nov 12, 2015 · 9 comments
Labels
actionable Clear and specific issues ready for anyone to take them. good first issue Ideal for your first contribution! (some Symfony experience may be required) hasPR A Pull Request has already been submitted for this issue.

Comments

@ghost
Copy link

ghost commented Nov 12, 2015 

The AppBundle\Menu\Builder class extends Symfony\Component\DependencyInjection\ContainerAware that is deprecated since version 2.8, to be removed in 3.0. Use the ContainerAwareTrait instead.

Code from documentation:
http://symfony.com/doc/master/bundles/KnpMenuBundle/index.html#method-a-the-easy-way-yay

Maybe there are more code examples which should use ContainerAwareTrait.
@ghost
Copy link
Author

ghost commented Nov 12, 2015

rel KnpLabs/KnpMenuBundle#290

@xabbuh
Copy link
Member

xabbuh commented Nov 12, 2015

@JHGitty Did you find an example in the docs that is not related to the KnpMenuBundle or should we close here in favour of the issue you linked?

@ghost
Copy link
Author

ghost commented Nov 13, 2015

@xabbuh https://github.com/symfony/symfony-docs/search?utf8=%E2%9C%93&q=ContainerAware

@wouterj
Copy link
Member

wouterj commented Nov 13, 2015

Both cases should be changed to extend Symfony\Bundle\FrameworkBundle\Controller\Controller instead. Can you make that change @JHGitty? (as you should get the credits!)

@wouterj wouterj added good first issue Ideal for your first contribution! (some Symfony experience may be required) actionable Clear and specific issues ready for anyone to take them. labels Nov 13, 2015
@ghost
Copy link
Author

ghost commented Nov 13, 2015

Anybody can do it :-) I have not much time for it.

@stof
Copy link
Member

stof commented Nov 13, 2015

@wouterj no, they should not extend Controller. They are not controllers. They should use the trait

@wouterj
Copy link
Member

wouterj commented Nov 13, 2015

@stof unless I'm missing something, both search results are controllers.

@ghost
Copy link
Author

ghost commented Nov 13, 2015

@stof WouterJ is talking about these lines:
https://github.com/symfony/symfony-docs/search?utf8=%E2%9C%93&q=ContainerAware

Both are controllers: FriendMessageController and MainController.

@stof
Copy link
Member

stof commented Nov 13, 2015

ah sorry, indeed

@javiereguiluz javiereguiluz mentioned this issue Nov 17, 2015
Merged
@xabbuh xabbuh added the hasPR A Pull Request has already been submitted for this issue. label Nov 20, 2015
xabbuh added a commit that referenced this issue Nov 26, 2015
@xabbuh
minor #5888 Removed the use of ContainerAware class (javiereguiluz)
400873c 
This PR was merged into the 2.3 branch.

Discussion
----------

Removed the use of ContainerAware class

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | #5884

Removing the use of `ContainerAware` in these examples required a heavy rewording. Please, review them. Thanks!

Commits
-------

b0cae3b Removed the use of ContainerAware class
@xabbuh xabbuh closed this as completed Nov 26, 2015
eSlider added a commit to mapbender/fom that referenced this issue Aug 28, 2017
@eSlider
Fix deprecation in FailedLoginListener (symfony/symfony-docs#5884)
cbcde51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
actionable Clear and specific issues ready for anyone to take them. good first issue Ideal for your first contribution! (some Symfony experience may be required) hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@stof @wouterj @xabbuh