Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Workflow] Fix eventsToDispatch parameter setup for StateMachine#44510

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

Conversation

@goodjinny
Copy link
Contributor

@goodjinnygoodjinny commentedDec 8, 2021
edited by ogizanagi
Loading

QA
Branch?5.3
Bug fix?yes
New feature?no
Deprecations?no
Ticketssee description
LicenseMIT
Doc PRN/A

Usingstate_machine type of the workflow in the project have mentioned that workflow events still propagated withevents_to_dispatch: [] parameter. But this bug does not appear forworkflow type.

Current pull request fixes that bug by adding $eventsToDispatch parameter initialization for StateMachine class.

Before fixing (state_machine withevents_to_dispatch: []):
Screenshot from 2021-12-08 10-55-51

After fixing (state_machine withevents_to_dispatch: []);
Screenshot from 2021-12-08 10-54-58

@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (seehttps://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
  • Features and deprecations must be submitted against the 6.1 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@goodjinnygoodjinny changed the base branch from5.4 to4.4December 8, 2021 17:45
@goodjinnygoodjinny changed the base branch from4.4 to5.4December 8, 2021 17:46
@goodjinny
Copy link
ContributorAuthor

goodjinny commentedDec 8, 2021
edited
Loading

I have found out that the lowest branch where this fix can be applied is 5.2. Should I close this PR and create the new one for the 5.2 branch?

@chalasr
Copy link
Member

5.2 is not maintained anymore, so 5.3 is probably the lowest maintained branch where this applies.
Assuming you have a remote pointing to this repo namedupstream, you can just change your base branch using theEdit button on your PR title, then on your machine rungit rebase --onto=upstream/5.3 on your branch and finallygit push --force.

@goodjinnygoodjinny changed the base branch from5.4 to5.3December 8, 2021 18:34
@goodjinnygoodjinnyforce-pushed thefix/workflow-events-to-dispatch-parameter-setup-for-state-machine branch fromce8e74e toca38501CompareDecember 8, 2021 18:42
@goodjinny
Copy link
ContributorAuthor

goodjinny commentedDec 8, 2021
edited
Loading

Thank you@chalasr! Done.

Could you review it again?

@ogizanagiogizanagi modified the milestones:5.4,5.3Dec 8, 2021
@fabpot
Copy link
Member

Thank you@goodjinny.

goodjinny reacted with thumbs up emoji

@fabpotfabpot merged commit5e954d6 intosymfony:5.3Dec 15, 2021
@fabpotfabpot mentioned this pull requestDec 29, 2021
This was referencedDec 29, 2021
@DoctorBryson
Copy link

After updating to 5.4.2 this update prevented the state machine to work at all, broke in our integration tests. Got to revert to 5.4.0 until a fix is there I guess.

@fancyweb
Copy link
Contributor

Hello@DoctorBryson, please create a new issue with a reproducer, thanks.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@lyrixxlyrixxlyrixx approved these changes

@chalasrchalasrAwaiting requested review from chalasr

@dunglasdunglasAwaiting requested review from dunglas

@wouterjwouterjAwaiting requested review from wouterj

@xabbuhxabbuhAwaiting requested review from xabbuh

@ycerutoycerutoAwaiting requested review from yceruto

@jderussejderusseAwaiting requested review from jderusse

@OskarStarkOskarStarkAwaiting requested review from OskarStark

Assignees

No one assigned

Projects

None yet

Milestone

5.3

Development

Successfully merging this pull request may close these issues.

8 participants

@goodjinny@carsonbot@chalasr@fabpot@DoctorBryson@fancyweb@lyrixx@ogizanagi

[8]ページ先頭

©2009-2025 Movatter.jp