Skip to content
This repository has been archived by the owner on Nov 25, 2020. It is now read-only.

Allow injecting the service into Controller constructor for autowiring #90

Merged
merged 3 commits into from
May 1, 2018

Conversation

andrewmy
Copy link
Contributor

This update allows injecting the service into Controller constructor for autowiring.

Copy link
Contributor

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, but we need to make that service private from now and make its alias public only

@@ -6,11 +6,13 @@
<services>

<!-- Our service, for controllers -->
<service id="white_october_breadcrumbs" class="WhiteOctober\BreadcrumbsBundle\Model\Breadcrumbs" public="true">
<service id="WhiteOctober\BreadcrumbsBundle\Model\Breadcrumbs" class="WhiteOctober\BreadcrumbsBundle\Model\Breadcrumbs" public="true">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be private from now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks

<call method="setRouter">
<argument type="service" id="router" />
</call>
</service>
<service id="white_october_breadcrumbs" alias="WhiteOctober\BreadcrumbsBundle\Model\Breadcrumbs">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should remain public

@andrewmy
Copy link
Contributor Author

Ready for merge?

Copy link
Contributor

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@andrewmy
Copy link
Contributor Author

andrewmy commented Mar 8, 2018

Cool! Can't wait for a point release ;)

@bocharsky-bw
Copy link
Contributor

For a possible workaround, until it's merged, you can add an alias in your system so you can use it for now:

# config/services.yaml
services:
    # ...
    WhiteOctober\BreadcrumbsBundle\Model\Breadcrumbs: '@white_october_breadcrumbs'

@sampart sampart merged commit 07ef1a3 into sampart:master May 1, 2018
@sampart
Copy link
Owner

sampart commented May 1, 2018

Thank you for this, @andrewmy. Sorry for the delay in merging.

Thank you @bocharsky-bw for reviewing and approving as well - makes my job a lot easier!

@sampart
Copy link
Owner

sampart commented May 1, 2018

This is now in release https://github.com/whiteoctober/BreadcrumbsBundle/releases/tag/1.4.2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants