Skip to content

[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

Merged
merged 1 commit into from
Mar 22, 2019

Conversation

lyrixx
Copy link
Member

@lyrixx 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

Copy link
Contributor

@Korbeil Korbeil left a 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 ?

@lyrixx lyrixx changed the title Added support for many inital places [Workflow] Added support for many inital places Mar 12, 2019
@lyrixx
Copy link
Member Author

lyrixx commented Mar 12, 2019

Also, is there a reason you don't throw an Exception when we have multiple initial places with single_state marking store ?

I will do that once #29457 is merged. It will be much more easier

@lyrixx lyrixx force-pushed the w-mutli-init-places branch from f623102 to 9a9184d Compare March 13, 2019 15:46
@lyrixx
Copy link
Member Author

lyrixx commented Mar 13, 2019

Here we Go. I updated validators and added test 👍

Copy link
Contributor

@Korbeil Korbeil left a comment

Choose a reason for hiding this comment

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

Thanks ! 👍

@lyrixx lyrixx force-pushed the w-mutli-init-places branch 2 times, most recently from 9c10c81 to 56c6090 Compare March 13, 2019 16:23
@lyrixx lyrixx force-pushed the w-mutli-init-places branch 2 times, most recently from a3ca1b6 to 66119d5 Compare March 20, 2019 19:14
@lyrixx lyrixx force-pushed the w-mutli-init-places branch from 66119d5 to 1af1bf2 Compare March 20, 2019 19:28
@lyrixx lyrixx merged commit 1af1bf2 into symfony:master Mar 22, 2019
lyrixx added a commit that referenced this pull request Mar 22, 2019
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
@lyrixx lyrixx deleted the w-mutli-init-places branch March 22, 2019 14:52
lyrixx added a commit to lyrixx/symfony that referenced this pull request Apr 6, 2019
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()`
lyrixx added a commit to lyrixx/symfony that referenced this pull request Apr 6, 2019
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()`
fabpot added a commit that referenced this pull request Apr 6, 2019
…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
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@fabpot fabpot mentioned this pull request May 9, 2019
lyrixx added a commit to lyrixx/symfony that referenced this pull request Jun 28, 2019
lyrixx added a commit to lyrixx/symfony that referenced this pull request Jun 28, 2019
lyrixx added a commit to lyrixx/symfony that referenced this pull request Jun 28, 2019
lyrixx added a commit to lyrixx/symfony that referenced this pull request Jun 28, 2019
lyrixx added a commit to lyrixx/symfony that referenced this pull request Jun 28, 2019
lyrixx added a commit that referenced this pull request Jun 28, 2019
…() (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()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants