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] Triggerentered event for subject entering in the Workflow for the first time#29145

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

Merged

Conversation

@lyrixx
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#28319
LicenseMIT
Doc PR

alexislefebvre, paulandrieux, and lenybernard reacted with thumbs up emoji
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

please submit a doc PR if applicable

@nicolas-grekas
Copy link
Member

Thank you@lyrixx.

@nicolas-grekasnicolas-grekas merged commit388840f intosymfony:masterDec 1, 2018
nicolas-grekas added a commit that referenced this pull requestDec 1, 2018
…g in the Workflow for the first time (lyrixx)This PR was merged into the 4.3-dev branch.Discussion----------[Workflow] Trigger `entered` event for subject entering in the Workflow for the first time| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#28319| License       | MIT| Doc PR        |Commits-------388840f [Workflow] Trigger `entered` event for subject entering in the Workflow for the first time
@lyrixxlyrixx deleted the workflow-initial-place branchDecember 3, 2018 17:28
@nicolas-grekasnicolas-grekas modified the milestones:next,4.3Apr 30, 2019
@fabpotfabpot mentioned this pull requestMay 9, 2019
@lstrojny
Copy link
Contributor

lstrojny commentedApr 28, 2020•
edited
Loading

I think this should be considered a BC break since it changes the signature ofWorkflow\Event::getTransition() fromgetTransition(): Transition togetTransition(): Transition|null.

@lyrixx
Copy link
MemberAuthor

There was not typehint nor PHPDoc on this method, so I guess this was not a BC break.
More over, this has been merged 2 year ago and you are the first one to report that, so I guess this is fine.

@lstrojny
Copy link
Contributor

The argument that a lack of PHPDoc makes it not a BC break doesn’t sound convincing to me as one wouldn't realistically expect users to assumegetTransition(): mixed before Symfony 4.3. And yes, it’s probably too late anyway to do anything about it. I don't know what the practice it but adding it to the 4.3 upgrade guide might be a good idea.

ctrl-f5 reacted with thumbs up emoji

@xabbuh
Copy link
Member

I think adding this to the upgrade file is a good idea as changing this again is too late.@lstrojny Would you like to send a PR changing theUPGRADE-4.3.md file (on the4.4` branch)?

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

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@xabbuhxabbuhxabbuh approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

5 participants

@lyrixx@nicolas-grekas@lstrojny@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp