Skip to content

Conversation

@mbabker
Copy link
Contributor

@mbabker mbabker commented Sep 1, 2020

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets N/A
License MIT

In winzouStateMachineBundle 0.4, the sm.factory, sm.callback_factory, and sm.callback.cascade_transition IDs are aliases and not real services. So, the aliases need to be made public as well otherwise they won't be available through the container once it's compiled.

@mbabker mbabker requested a review from a team as a code owner September 1, 2020 00:41
@@ -39,9 +38,12 @@ public function process(ContainerBuilder $container): void
$container->setAlias('sm.callback.cascade_transition', CascadeTransitionCallback::class);
Copy link
Contributor

@stloyd stloyd Sep 2, 2020

Choose a reason for hiding this comment

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

IMHO this should be changed to just like(tested the current proposal & it didn't work for me):

Suggested change
$container->setAlias('sm.callback.cascade_transition', CascadeTransitionCallback::class);
$container->setAlias('sm.factory', FactoryInterface::class)->setPublic(true);
$container->setAlias('sm.callback_factory', CallbackFactoryInterface::class)->setPublic(true);
$container->setAlias('sm.callback.cascade_transition', CascadeTransitionCallback::class)->setPublic(true);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR seems fine as is for me, this is coming out of my local Sylius clone with my WIP upgrade PR applied:

Michaels-MacBook-Pro-2:Sylius mbabker$ bin/console debug:container sm.factory

 // This service is a public alias for the service SM\Factory\Factory

Compared to the current state of affairs:

Michaels-MacBook-Pro-2:Sylius mbabker$ bin/console debug:container sm.factory

 // This service is a private alias for the service SM\Factory\Factory                                                  

... snip ...

 ! [NOTE] The "sm.factory" service or alias has been removed or inlined when the container was compiled.

@lchrusciel
Copy link
Contributor

We have to fix the main build before we can go anywhere further.

@lchrusciel lchrusciel linked an issue Sep 9, 2020 that may be closed by this pull request
@pamil
Copy link
Contributor

pamil commented Oct 14, 2020

Rebased & added a functional test for these services accessibility.

@pamil pamil merged commit 5e94583 into Sylius:master Oct 14, 2020
@pamil
Copy link
Contributor

pamil commented Oct 14, 2020

Thank you, Michael! 🥇

@mbabker mbabker deleted the sm-bc-tweak branch October 14, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Symfony 5 support

4 participants