-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Workflow] Added support for many inital places #30468
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
Conversation
lyrixx
commented
Mar 6, 2019
Q | A |
---|---|
Branch? | master |
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | yes |
Tests pass? | yes |
Fixed tickets | #30080 |
License | MIT |
Doc PR |
35880b2
to
f623102
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR, this will help us improve how we use workflow 😄
Quick notice, I did not see tests with multiple initial places, I think it should be better to add atleast one.
Also, is there a reason you don't throw an Exception when we have multiple initial places with single_state
marking store ?
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
Outdated
Show resolved
Hide resolved
I will do that once #29457 is merged. It will be much more easier |
f623102
to
9a9184d
Compare
Here we Go. I updated validators and added test 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks ! 👍
9c10c81
to
56c6090
Compare
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Outdated
Show resolved
Hide resolved
a3ca1b6
to
66119d5
Compare
66119d5
to
1af1bf2
Compare
This PR was merged into the 4.3-dev branch. Discussion ---------- [Workflow] Added support for many inital places | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #30080 | License | MIT | Doc PR | Commits ------- 1af1bf2 Added support for many inital places
I introduced a BC break in symfony#30468 and this PR fix it. With the full stack framework, when one does not configure the initial_place(s) the DIC set `[]` for the initial values. So it removes the initials values guessed in `Definition::addPlace()`
I introduced a BC break in symfony#30468 and this PR fix it. With the full stack framework, when one does not configure the initial_place(s) the DIC set `[]` for the initial values. So it removes the initials values guessed in `Definition::addPlace()`
…red (lyrixx) This PR was merged into the 4.3-dev branch. Discussion ---------- [Workflow] Fixed initial places when no places are configured | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | EUFOSSA I introduced a BC break in #30468 and this PR fix it. With the full stack framework, when one does not configure the initial_place(s) the DIC set `[]` for the initial values. So it removes the initials values guessed in `Definition::addPlace()` Commits ------- 76fd9c3 [Workflow] Fixed initial places when no places are configured
Added missing part of symfony#30468
Added missing part of symfony#30468
Added missing part of symfony#30468
Added missing part of symfony#30468
Added missing part of symfony#30468
…() (lyrixx) This PR was merged into the 4.3 branch. Discussion ---------- [Workflow] Deprecated DefinitionBuilder::setInitialPlace() Added missing part of #30468 | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Commits ------- 4d002e8 [Workflow] Deprecated DefinitionBuilder::setInitialPlace()