Skip to content

[DependencyInjection] Add section about Service Closures #15730

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

Closed
wants to merge 1 commit into from

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented Aug 19, 2021

Documents service closures added in symfony/symfony#21770 and symfony/symfony#41176.

@carsonbot carsonbot added this to the 4.4 milestone Aug 19, 2021
@HypeMC HypeMC force-pushed the service-closure branch 3 times, most recently from 374088f to 0da6fc7 Compare August 20, 2021 00:58
nicolas-grekas added a commit to symfony/symfony that referenced this pull request Aug 20, 2021
…-DSL (HypeMC)

This PR was merged into the 5.4 branch.

Discussion
----------

[DependencyInjection] Add service_closure() to the PHP-DSL

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | symfony/symfony-docs#15730 and symfony/symfony-docs#15731

Adds a `service_closure()` function to the PHP-DSL.

Commits
-------

f333fa0 [DI] Add service_closure() to the PHP-DSL
symfony-splitter pushed a commit to symfony/dependency-injection that referenced this pull request Aug 20, 2021
…-DSL (HypeMC)

This PR was merged into the 5.4 branch.

Discussion
----------

[DependencyInjection] Add service_closure() to the PHP-DSL

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | symfony/symfony-docs#15730 and symfony/symfony-docs#15731

Adds a `service_closure()` function to the PHP-DSL.

Commits
-------

f333fa0e05 [DI] Add service_closure() to the PHP-DSL
Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

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

Thank you for documenting this!

Comment on lines 22 to 25
/**
* @var \Closure
*/
private $mailer;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/**
* @var \Closure
*/
private $mailer;
/**
* @var callable(): MailerInterface
*/
private \Closure $mailer;

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

$services = $configurator->services();

$services->set(MyService::class)
->args([new ServiceClosureArgument(new Reference('mailer'))]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to mergers: this should be updated to:

Suggested change
->args([new ServiceClosureArgument(new Reference('mailer'))]);
->args([service_closure('mailer')]);

when merged up in 5.4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a separate PR for that #15731

@HypeMC HypeMC force-pushed the service-closure branch 2 times, most recently from 722e76e to 73125ee Compare October 1, 2022 18:43
@HypeMC HypeMC force-pushed the service-closure branch from 73125ee to 734831b Compare March 2, 2023 08:37
@HypeMC HypeMC force-pushed the service-closure branch from 734831b to 70d745a Compare March 2, 2023 08:50
@OskarStark
Copy link
Contributor

As 4.4 is not maintained anymore we should only work on 5.4, can you merge the two PRs?

@HypeMC HypeMC closed this Mar 2, 2023
@HypeMC HypeMC deleted the service-closure branch March 2, 2023 11:58
@HypeMC
Copy link
Contributor Author

HypeMC commented Mar 2, 2023

@OskarStark Done, #15731 should be good to go.

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

Successfully merging this pull request may close these issues.

4 participants