Skip to content

Changed property visibility to protected #321

Closed
wants to merge 1 commit into from

3 participants

@barnabywalters

The main reason being to allow QueryBuilder to be subclassed — for example so that we can fix issues like problematic shallow cloning locally and use them without having to wait for a pull request to be merged.

@barnabywalters barnabywalters Changed property visibility to protected
The main reason being to allow QueryBuilder to be subclassed — for example so that we can fix issues like problematic shallow cloning locally and use them without having to wait for a pull request to be merged.
8e446f7
@stof
Doctrine member
stof commented May 20, 2013

Changing the visibility of all these properties means that they become extension points for which we then must provide BC until 3.0 according to our guidelines. So IMO, it is a -1 here.

@Ocramius
Doctrine member

@barnabywalters the QueryBuilder is already very complex - as @stof said, you're also locking it from being changed internally, and that's problematic right now, since internal refactorings can happen quite often because of its complexity.

@Ocramius Ocramius closed this May 20, 2013
@barnabywalters

Understood, thanks for the explanations :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.