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

[FrameworkBundle] deal with explicitly enabled workflow nodes#28769

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
lyrixx merged 1 commit intosymfony:3.4fromxabbuh:issue-28662
Nov 19, 2018

Conversation

@xabbuh
Copy link
Member

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?not yet
Fixed tickets#28662
LicenseMIT
Doc PR

http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

<framework:config>
<framework:workflow enabled="true" name="foo" type="workflow" initial-place="bar">
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

This test currently does not pass. For the other formats the attribute is not configured per workflow but only once. How do we want to handle it in XML? Completely forbidding it? Requiring it to have the same value for each workflow if the attribute is set?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

@symfony/deciders What is your opinion here? :)

Copy link
Member

@nicolas-grekasnicolas-grekasOct 31, 2018
edited
Loading

Choose a reason for hiding this comment

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

actually what's the meaning of the "enabled" option on the configuration?
as soon as one workflow is defined, workflow is enabled - and if no workflows are defined, it's disabled.
Wouldn't this make sense?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Generally yes, this option was introduced to be able to explicitly enable the workflow services while not using the config to define your workflows (see#24051 for the context).

Copy link
Member

Choose a reason for hiding this comment

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

XML are boring for the framework configuration.

I would like instead close#28662 as won't fix. Use should use the "regular" way to define workflow.

Copy link
Member

Choose a reason for hiding this comment

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

Anyway, I would go for supporting
<framework:workflow enabled="true" />
or your current exemple withoutenabled="true"
and that's all

@xabbuh
Copy link
MemberAuthor

The extension now throws whenenabled="false is present for a single workflow.

Status: Needs Review

@xabbuhxabbuh changed the title[WIP][FrameworkBundle] deal with explicitly enabled workflow nodes[FrameworkBundle] deal with explicitly enabled workflow nodesNov 16, 2018
@lyrixx
Copy link
Member

... and one more bug fixed, thank you Christian.

@lyrixxlyrixx merged commit017fd56 intosymfony:3.4Nov 19, 2018
lyrixx added a commit that referenced this pull requestNov 19, 2018
…des (xabbuh)This PR was merged into the 3.4 branch.Discussion----------[FrameworkBundle] deal with explicitly enabled workflow nodes| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | not yet| Fixed tickets |#28662| License       | MIT| Doc PR        |Commits-------017fd56 deal with explicitly enabled workflow nodes
@xabbuhxabbuh deleted the issue-28662 branchNovember 19, 2018 10:39
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

@lyrixxlyrixxlyrixx approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

4 participants

@xabbuh@lyrixx@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp