Skip to content

[Workflow] Choose which Workflow events should be dispatched #37815

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 2 commits into from
Aug 13, 2020

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Aug 12, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #36633 ; Fix #36617
License MIT
Doc PR

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

LGTM, with minor comments about the naming.

@lyrixx
Copy link
Member Author

lyrixx commented Aug 13, 2020

@fabpot thanks for the review. I have addressed your comments

@lyrixx lyrixx force-pushed the event-dispatched branch 2 times, most recently from c739bea to 1112937 Compare August 13, 2020 07:03
@fabpot
Copy link
Member

fabpot commented Aug 13, 2020

Thank you @lyrixx.

@fabpot fabpot merged commit f3962d4 into symfony:master Aug 13, 2020
@lyrixx lyrixx deleted the event-dispatched branch August 13, 2020 08:14
fabpot added a commit that referenced this pull request Aug 13, 2020
This PR was merged into the 5.2-dev branch.

Discussion
----------

[Workflow] Improve and fix

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #37815 review
| License       | MIT
| Doc PR        | #

Hi @lyrixx
thx for such fast PR take over :)

while reviewing i found this typo i think
also i reordered the constants inside the events class to map with the order of fireing event

Commits
-------

9bbce41 [Workflow] Improve and fix
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Aug 13, 2020
This PR was merged into the 5.2-dev branch.

Discussion
----------

[Workflow] Improve and fix

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix symfony/symfony#37815 review
| License       | MIT
| Doc PR        | #

Hi @lyrixx
thx for such fast PR take over :)

while reviewing i found this typo i think
also i reordered the constants inside the events class to map with the order of fireing event

Commits
-------

9bbce417ff [Workflow] Improve and fix
@fabpot fabpot mentioned this pull request Oct 5, 2020
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.

[Workflow] Choose which Workflow events should be dispatched
4 participants