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
Appearance settings

[DoctrineBridge] Fixed bc layer after #18069 #18677

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 11, 2016
Merged

[DoctrineBridge] Fixed bc layer after #18069 #18677

merged 1 commit into from
May 11, 2016

Conversation

HeahDude
Copy link
Contributor

Q A
Branch? master
Bug fix? BC break
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ~
License MIT
Doc PR ~

@HeahDude
Copy link
Contributor Author

HeahDude commented Apr 30, 2016 •
edited
Loading

I've got that weird feeling after #18069 got merged, it's now clear:

while addressing @xabbuh's comment about someone calling parent::onBind(), I added the condition __CLASS__ === get_class($this) to fix the triggering of the deprecation, but I guess that also breaks the call because the logic now in onSubmit() is not excecuted.

This PR adds another flag to fix this BC break.

Sorry for this confusion.

@HeahDude
Copy link
Contributor Author

HeahDude commented May 3, 2016

Any thought on this BC break ?

@xabbuh
Copy link
Member

xabbuh commented May 3, 2016

I think a test for this would be good.

@HeahDude HeahDude force-pushed the fix/bc_layer-18069 branch from 81f1a80 to 5683718 Compare May 3, 2016 22:55
@HeahDude
Copy link
Contributor Author

HeahDude commented May 3, 2016

Thanks @xabbuh I've added a failing test which revealed another issue, we have to keep the event in onBind() definition or it ends with a PHP error...
I also added the missing Listener suffix to the test class name.

Note that the patch works as expected, tests are green now :)

@HeahDude HeahDude changed the title [DoctrineBridge] Added a bc layer for #18069 [DoctrineBridge] Fixed bc layer after #18069 May 3, 2016
@HeahDude HeahDude force-pushed the fix/bc_layer-18069 branch from 5683718 to 8a6cf9d Compare May 3, 2016 23:20
@fabpot
Copy link
Member

fabpot commented May 11, 2016

Thank you @HeahDude.

@fabpot fabpot merged commit 8a6cf9d into symfony:master May 11, 2016
fabpot added a commit that referenced this pull request May 11, 2016
@fabpot
bug #18677 [DoctrineBridge] Fixed bc layer after #18069 (HeahDude)
97ebca1 
This PR was merged into the 3.1-dev branch.

Discussion
----------

[DoctrineBridge] Fixed bc layer after #18069

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | BC break
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ~
| License       | MIT
| Doc PR        | ~

Commits
-------

8a6cf9d [DoctrineBridge] fixed bc layer from #18069
@HeahDude HeahDude deleted the fix/bc_layer-18069 branch May 12, 2016 09:26
@fabpot fabpot mentioned this pull request May 13, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
BC Break DoctrineBridge Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@HeahDude @xabbuh @fabpot @javiereguiluz @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.