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

[Workflow] Mark registry as internal and deprecate the service#46000

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
fabpot merged 1 commit intosymfony:6.2fromlyrixx:w-registry
Aug 12, 2022

Conversation

@lyrixx
Copy link
Member

@lyrixxlyrixx commentedApr 11, 2022
edited
Loading

QA
Branch?6.2
Bug fix?no
New feature?yes
Deprecations?yes
TicketsFix#45315
LicenseMIT
Doc PR

The Registry was added to be able to get a workflow easily from twig templates
without having to specify the workflow name.

Unfortunately, many people thought this was the only way to get a workflow and
we even add some flexibility to retrieve the workflow from the registry.

I think it's a bad idea to inject the registry, it has the same default as
injecting the Container: It's a black box, does not respect SOLID principle and
the demeter law, and make testing harder.

So I decided to mark the registry as internal. Instead people have to inject the
"right" workflow where they need it.

It can be legit to need all workflows, for documentation or for testing.
So, all workflow services are tagged and can be injected with the
following YAML syntax:

!tagged_locator  { tag: workflow, index_by: name }

or PHP syntax:

tagged_locator('workflow','name')

Also, two others tags exists for each workflow types

  • workflow.workflow
  • workflow.state_machine

94noni and mvhirsch 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.

Makes sense to me.
we should also deprecate the corresponding autowiring alias:
src/Symfony/Bundle/FrameworkBundle/Resources/config/workflow.php: ->alias(Registry::class, 'workflow.registry')

@chalasrchalasr modified the milestones:6.1,6.2Apr 18, 2022
@fabpot
Copy link
Member

@lyrixx Do you have time to finish this one?

@lyrixx
Copy link
MemberAuthor

Yes, I'll do that ASAP, But I'm on holidays for 2 weeks. I'll do that after!

@lyrixxlyrixx changed the title[Workflow] MarkRegistry as internal and add a ServiceLocator[Workflow] Mark registry as internal and deprecate the serviceAug 12, 2022
@lyrixx
Copy link
MemberAuthor

Hello,

I pushed a new version without the service locator. I also updated the PR description with the new usage

Instead, all workflow services are tagged and can be injected with thefollowing YAML syntax:```yaml!tagged_locator  { tag: workflow, index_by: name }```or PHP syntax:```phptagged_locator('workflow', 'name')```Also, two others tags exists for each workflow types* `workflow.workflow`* `workflow.state_machine`
@fabpot
Copy link
Member

Thank you@lyrixx.

lyrixx reacted with heart emoji

@fabpotfabpot merged commit15ee67e intosymfony:6.2Aug 12, 2022
@lyrixxlyrixx deleted the w-registry branchAugust 12, 2022 12:03
fabpot added a commit that referenced this pull requestAug 19, 2022
This PR was merged into the 6.2 branch.Discussion----------[FrameworkBundle] clean up unused variables| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       || License       | MIT| Doc PR        |spotted while reviewing#46000Commits-------ccfb1e5 clean up unused variables
@fabpotfabpot mentioned this pull requestOct 24, 2022
@tacman
Copy link
Contributor

I'm in the category of programmers that injected the workflow registry in order to get a list of workflows in a helper bundle for workflow.

// now deprecated, following https://symfony.com/doc/current/workflow.html#accessing-the-workflow-in-a-classclass WorkflowHelperService{publicfunction __construct(privateRegistry$workflowRegistry,// in loadExtension of the bundle:$builder->autowire(WorkflowHelperService::class)            ->setArgument('$workflowRegistry',new Reference('workflow.registry'));
// better wayclass WorkflowHelperService{publicfunction __construct(privateServiceLocator$locator,// loadExtension()$builder->autowire(WorkflowHelperService::class)            ->setArgument('$locator', tagged_locator(tag: 'workflow', indexAttribute: 'name' ))            ->setAutoconfigured(true)        ;

Using this, I'm not getting any services in the locator, $this->locator->count() is zero.

I imagine I'm not using tagger_locator correctly, or I'm doing this in the wrong spot (I'm doing this in loadExtension of my bundle, perhaps it needs to be done in the compiler pass? If so, could you point me in the right direction on how to do that, I still find the bundle interactions a bit intimidating.

@tacman
Copy link
Contributor

Also, is this the right spot for that question? The issue itself is closed, should I open a new one?

@stof
Copy link
Member

You should open a new discussion

lyrixx added a commit to lyrixx/symfony-docs that referenced this pull requestFeb 7, 2023
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestFeb 21, 2023
This PR was merged into the 6.2 branch.Discussion----------[Workflow] Do not talk about registryThis is not recommended sincesymfony/symfony#46000Commits-------e169f78 [Workflow] Do not talk about registry
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@fabpotfabpotfabpot approved these changes

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

+1 more reviewer

@94noni94noni94noni left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

[Workflow] Allow fetching workflow (definition) by name from registry

8 participants

@lyrixx@fabpot@tacman@stof@nicolas-grekas@94noni@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp