Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Workflow] Fixed support of multiple transitions with the same name.#21280
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
MamWayne commentedJan 13, 2017
Well done ;) |
ecb2992 to53938a5Compare
Nyholm left a comment
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.
I like this PR but I think it is missing one test.
Current marking: {a, b}.
You have the following transitions: t1: a->c, t2: b->d. (Both transitions named "foobar")
You should check that {a,b} -> ["foobar"] -> {c,d}.
53938a5 toe80b866Comparelyrixx commentedJan 14, 2017
@Nyholm I added a new test case. |
The previous behavior was underterministic because it took the firsttransition during the `can` and the `apply` method. But the "first"does not mean anything. Now the workflow apply all possibletransitions with the same name.
e80b866 toedd5431Comparenicolas-grekas commentedJan 14, 2017
👍 |
Nyholm left a comment
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.
Awesome!
fabpot commentedJan 15, 2017
This is not strictly a bug fix. I understand that the current behavior is not the "right"/"expected" one, but I don't think this qualifies as something for 3.2. I would recommend to merge it in 3.3 instead. @lyrixx Can you add a note in the component CHANGELOG about this change? |
lyrixx commentedJan 16, 2017
IMHO, it should go on 3.2 because it will be impossible for 3.2 users to have the correct behavior, and then when they will upgrade to 3.3, the behavior will be different than what they know. |
lyrixx commentedJan 16, 2017
Please note that there are not new visible interface, so it's a pure behavioral change |
fabpot commentedJan 18, 2017
Thank you@lyrixx. |
…same name. (lyrixx)This PR was merged into the 3.2 branch.Discussion----------[Workflow] Fixed support of multiple transitions with the same name.| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | ----The previous behavior was underterministic because it took the firsttransition during the `can` and the `apply` method. But the "first"does not mean anything. Now the workflown apply all possible transitionswith the same name.Commits-------edd5431 [Workflow] Fixed support of multiple transition with the same name.
Uh oh!
There was an error while loading.Please reload this page.
The previous behavior was underterministic because it took the first
transition during the
canand theapplymethod. But the "first"does not mean anything. Now the workflown apply all possible transitions
with the same name.