[FrameworkBundle] [Workflow] fixed initial place config #20490

HeahDude · 2016-11-11T15:31:14Z

Q	A
Branch?	master
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#20458 (comment)
License	MIT
Doc PR	~

xabbuh · 2016-11-11T16:41:40Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -238,6 +238,7 @@
        <xsd:sequence>
            <xsd:element name="marking-store" type="marking_store" />
            <xsd:element name="supports" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
+            <xsd:element name="initial_place" type="xsd:string" />


should be <xsd:element name="initial_place" type="xsd:string" minOccurs="0" maxOccurs="1" />

Thank you fixed.

HeahDude · 2016-11-11T17:05:42Z

I've also fixed the test https://github.com/symfony/symfony/pull/20490/files#diff-aa781e95d1e8284ae37b68430bb2128bR31.

xabbuh · 2016-11-12T09:46:33Z

src/Symfony/Component/Workflow/Validator/WorkflowValidator.php

@@ -20,5 +20,6 @@ class WorkflowValidator implements DefinitionValidatorInterface
 {
    public function validate(Definition $definition, $name)
    {
+        return true;


I think I tend to agree with @ro0NL that this method shouldn't return anything. Why is that method empty in the WorkfowValidator at all?

Ok I just remove this extra commit to keep the focus on the config fix in this PR.

xabbuh · 2016-11-12T10:02:30Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+                    'type' => $type,
+                    'marking_store' => isset($workflow['marking_store']['type']) ? $workflow['marking_store']['type'] : null,
+                ))
+            ;


Not sure if it's really worth making this change. It makes git blame pretty useless when you try to figure out when a certain change was introduced without much value.

I agree with @xabbuh

stof · 2016-11-12T11:38:41Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -238,6 +238,7 @@
        <xsd:sequence>
            <xsd:element name="marking-store" type="marking_store" />
            <xsd:element name="supports" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
+            <xsd:element name="initial_place" type="xsd:string" minOccurs="0" maxOccurs="1" />


should be an attribute, not a child tag

Fixed, thanks

xabbuh · 2016-11-12T16:20:52Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -235,6 +235,7 @@
    </xsd:complexType>

    <xsd:complexType name="workflow">
+        <xsd:attribute name="initial_place" type="xsd:string" />


I would move it after the sequence next to the already existing attribute.

Missed that, thanks!

lyrixx · 2016-11-15T09:55:48Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+                    'type' => $type,
+                    'marking_store' => isset($workflow['marking_store']['type']) ? $workflow['marking_store']['type'] : null,
+                ))
+            ;


I agree with @xabbuh

lyrixx · 2016-11-15T09:56:38Z

@HeahDude Could you finish this one? I want to merge it asap.

stof · 2016-11-15T10:01:27Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -242,6 +242,7 @@
            <xsd:element name="transitions" type="transitions" />
        </xsd:sequence>
        <xsd:attribute name="name" type="xsd:string" />
+        <xsd:attribute name="initial_place" type="xsd:string" />


the attribute name should use a dash in XML, not an underscore

HeahDude · 2016-11-15T14:05:51Z

@lyrixx done!

lyrixx · 2016-11-15T14:32:13Z

👍

lyrixx · 2016-11-15T14:33:18Z

Good catch, thanks @HeahDude.

…(HeahDude) This PR was merged into the 3.2-dev branch. Discussion ---------- [FrameworkBundle] [Workflow] fixed initial place config | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20458 (comment) | License | MIT | Doc PR | ~ Commits ------- 91237c9 [FrameworkBundle] [Workflow] Fixed initial place config

carsonbot added the Status: Needs Review label Nov 11, 2016

HeahDude changed the title ~~Fix workflow initial place config~~ [FrameworkBundle] [Workflow] minor config and validator fixes Nov 11, 2016

HeahDude mentioned this pull request Nov 11, 2016

Added XML support for Workflow configuration #20458

Merged

HeahDude force-pushed the fix-workflow-initial-place-config branch from 2805702 to f2b36b1 Compare November 11, 2016 15:42

xabbuh requested changes Nov 11, 2016

View reviewed changes

HeahDude force-pushed the fix-workflow-initial-place-config branch from f2b36b1 to 94c6d44 Compare November 11, 2016 16:45

xabbuh approved these changes Nov 11, 2016

View reviewed changes

HeahDude force-pushed the fix-workflow-initial-place-config branch from 94c6d44 to b83f547 Compare November 11, 2016 17:04

ro0NL mentioned this pull request Nov 11, 2016

[Workflow] Clarify validator API + fixed unknown "scalar" marking store #20492

Closed

xabbuh reviewed Nov 12, 2016

View reviewed changes

HeahDude force-pushed the fix-workflow-initial-place-config branch from b83f547 to c0d54ef Compare November 12, 2016 09:55

HeahDude changed the title ~~[FrameworkBundle] [Workflow] minor config and validator fixes~~ [FrameworkBundle] [Workflow]fixed initial place config Nov 12, 2016

HeahDude changed the title ~~[FrameworkBundle] [Workflow]fixed initial place config~~ [FrameworkBundle] [Workflow] fixed initial place config Nov 12, 2016

xabbuh reviewed Nov 12, 2016

View reviewed changes

stof reviewed Nov 12, 2016

View reviewed changes

HeahDude force-pushed the fix-workflow-initial-place-config branch from c0d54ef to a5a6a3e Compare November 12, 2016 16:16

xabbuh reviewed Nov 12, 2016

View reviewed changes

HeahDude force-pushed the fix-workflow-initial-place-config branch from a5a6a3e to 7081c92 Compare November 12, 2016 16:51

xabbuh added FrameworkBundle Workflow labels Nov 14, 2016

lyrixx requested changes Nov 15, 2016

View reviewed changes

stof requested changes Nov 15, 2016

View reviewed changes

[FrameworkBundle] [Workflow] Fixed initial place config

91237c9

HeahDude force-pushed the fix-workflow-initial-place-config branch from 7081c92 to 91237c9 Compare November 15, 2016 14:03

lyrixx approved these changes Nov 15, 2016

View reviewed changes

lyrixx merged commit 91237c9 into symfony:master Nov 15, 2016

HeahDude deleted the fix-workflow-initial-place-config branch November 15, 2016 14:33

Uh oh!

[FrameworkBundle] [Workflow] fixed initial place config #20490

[FrameworkBundle] [Workflow] fixed initial place config #20490

Uh oh!

Conversation

HeahDude commented Nov 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HeahDude commented Nov 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lyrixx commented Nov 15, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HeahDude commented Nov 15, 2016

Uh oh!

lyrixx commented Nov 15, 2016

Uh oh!

lyrixx commented Nov 15, 2016

Uh oh!

Uh oh!

HeahDude commented Nov 11, 2016 •

edited

Loading