[Workflow] Fixed support of multiple transitions with the same name. #21280
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Well done ;) |
ecb2992 to
53938a5
Compare
Nyholm
requested changes
Jan 14, 2017
53938a5 to
e80b866
Compare
|
@Nyholm I added a new test case. |
The previous behavior was underterministic because it took the first transition during the `can` and the `apply` method. But the "first" does not mean anything. Now the workflow apply all possible transitions with the same name.
e80b866 to
edd5431
Compare
|
👍 |
|
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? |
|
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. |
|
Please note that there are not new visible interface, so it's a pure behavioral change |
|
Thank you @lyrixx. |
fabpot
added a commit
that referenced
this pull request
Jan 18, 2017
…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 first transition during the `can` and the `apply` method. But the "first" does not mean anything. Now the workflown apply all possible transitions with the same name. Commits ------- edd5431 [Workflow] Fixed support of multiple transition with the same name.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.