Skip to content

[Workflow] Added more events to the announce function #23299

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

Closed
wants to merge 3 commits into from

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Jun 25, 2017

Q A
Branch? 3.2
Bug fix? yes/no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #23275
License MIT
Doc PR #8087

This PR will fix #23275

The documentation states that we dispatch events workflow.announce and workflow.[name].announce. It was me who wrongly added it to the docs... sorry about that.

We could either: Change the docs or add these events. I choose to add these event to the source since the same events are dispatched for "guard", "leave", "transition", "enter" and "entered".

Copy link
Contributor

@ostrolucky ostrolucky left a comment

Choose a reason for hiding this comment

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

Even if you chose the code change, remember that docs need to be updated, because they currently state "announce: Triggered once for each workflow that now is available for the object."

@Nyholm
Copy link
Member Author

Nyholm commented Jun 26, 2017

Thank you @ostrolucky for your review. I will send a PR to the docs.

Also, we need to make sure we do not "announce" before the guard events. The guard events may block the transition.

@Nyholm
Copy link
Member Author

Nyholm commented Jun 26, 2017

Sorry, I was wrong in my last comment and commit. I reverted it, the PR is now unchanged since you reviewed it.

I made a small PR to the docs to address the issue @ostrolucky highlighted.

@lyrixx
Copy link
Member

lyrixx commented Jun 26, 2017

Thanks for fixing this bug @Nyholm.

lyrixx added a commit that referenced this pull request Jun 26, 2017
…olm)

This PR was submitted for the master branch but it was merged into the 3.2 branch instead (closes #23299).

Discussion
----------

[Workflow] Added more events to the announce function

| Q             | A
| ------------- | ---
| Branch?       | 3.2
| Bug fix?      | yes/no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #23275
| License       | MIT
| Doc PR        | #8087

This PR will fix #23275

The documentation states that we dispatch events `workflow.announce` and `workflow.[name].announce`. It was me who wrongly added it to the docs... sorry about that.

We could either: Change the docs or add these events. I choose to add these event to the source since the same events are dispatched for "guard", "leave", "transition", "enter" and "entered".

Commits
-------

c5042f3 [Workflow] Added more events to the announce function
@lyrixx
Copy link
Member

lyrixx commented Jun 26, 2017

I merged this PR in the 3.2 branch instead of master.

@lyrixx lyrixx closed this Jun 26, 2017
@Nyholm
Copy link
Member Author

Nyholm commented Jun 26, 2017

Thank you for merging.

This was referenced Jul 4, 2017
xabbuh added a commit to symfony/symfony-docs that referenced this pull request Jul 4, 2017
This PR was submitted for the 3.3 branch but it was merged into the 3.2 branch instead (closes #8087).

Discussion
----------

[Workflow] Updated language for annouce event

Related to symfony/symfony#23299

Commits
-------

9b6dce4 Updated language for annouce event
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] Announcing doesn't work as advertised
5 participants