Skip to content
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

Fix return value of PersistentCollection#removeElement() for extra-lazy collections: should be bool #6550

Merged
merged 2 commits into from
Jul 22, 2017
Merged

Fix return value of PersistentCollection#removeElement() for extra-lazy collections: should be bool #6550

merged 2 commits into from
Jul 22, 2017

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Jul 13, 2017

Fixes #5745.

As indicated in the ticket, calling removeElement on an uninitialized collection that uses the extra_lazy fetch mode does not return a bool, breaking the contract specified in the collection interface. This PR changes removeElement to return a bool value in those cases as well.

@Ocramius Ocramius self-assigned this Jul 13, 2017
@Ocramius Ocramius removed their assignment Jul 13, 2017
Ocramius
Ocramius suggested changes Jul 13, 2017
View reviewed changes
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

@alcaeus an additional test for the false scenario (that was now removed, it was the null scenario) is needed.

The test would basically attempt to call removeElement() with a non-existing or unrelated entity.

@Ocramius Ocramius added this to the 2.5.7 milestone Jul 13, 2017
@Ocramius Ocramius mentioned this pull request Jul 13, 2017
Closed
@alcaeus
Copy link
Member Author

alcaeus commented Jul 13, 2017

@Ocramius done!

@Ocramius Ocramius self-assigned this Jul 22, 2017
@Ocramius
Copy link
Member

🚢

@Ocramius Ocramius merged commit 668ad4c into doctrine:master Jul 22, 2017
Ocramius added a commit that referenced this pull request Jul 22, 2017
@Ocramius
Merge branch 'fix/#6550-correct-return-value-of-extra-lazy-removeElem…
b3ceef0 
…ent-calls' into 2.5

Backport #6550 to 2.5
@Ocramius
Copy link
Member

Backported to 2.5 via b3ceef0, thanks!

@alcaeus alcaeus deleted the fix-persistent-collection-return branch July 24, 2017 07:22
@Ocramius Ocramius changed the title Fix return value of removeElement for extra lazy collections Fix return value of PersistentCollection#removeElement() for extra-lazy collections: should be bool Jul 27, 2017
@Ocramius Ocramius mentioned this pull request Sep 27, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ocramius Ocramius Ocramius requested changes

Assignees

@Ocramius Ocramius

Labels
Bug
Projects
None yet
Milestone
2.5.7
Development

Successfully merging this pull request may close these issues.
2 participants
@alcaeus @Ocramius