Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix forget typehint #346

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants

No description provided.

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-564

We use Jira to track the state of pull requests and the versions they got
included in.

Member

stof commented Jul 22, 2013

this is a BC break as it changes the signature of the method (btw, even DBAL itself is affected by the BC break, as catched by the testsuite on Travis)

Owner

Ocramius commented Jul 22, 2013

@stof could be ok for 2.5. Otherwise, agreed, the break is annoying at this stage.

Member

stof commented Jul 22, 2013

@Ocramius a BC break on the main public interface of the library just to add a typehint is not worth it IMO. To follow semver, a BC break should not happen in the public API in minor releases (for internal-only APIs, it depends of the decision of the project). And Doctrine\DBAL\Connection::fetchAll is clearly part of the public API.

Member

deeky666 commented Jul 26, 2013

@stof I agree. It doesn't matter whether fetchAll() or the within called executeQuery() throws a PHP error if an array is NOT given (as executeQuery() is array type hinted). This does not affect the behaviour and can be changed in 3.0.

Owner

beberlei commented Aug 10, 2013

BC break.

@beberlei beberlei closed this Aug 10, 2013

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