Skip to content

Bump PHPStan version #2267

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 23, 2018
Merged

Bump PHPStan version #2267

merged 2 commits into from
Oct 23, 2018

Conversation

antograssiot
Copy link
Contributor

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

and fix a mistake in a readme

Instead of adding a new exclude rule, I would rather drop this code and in other places in the QueryJoinParser. Those are public method that didn't moved since 6 years and are still there in the next 3.0, I fail to understand what it is trying to fix and why if the method is to be removed at some point, we expect the internal property to still be there.
let me know if it is ok

and fix a mistake in a readme
@dunglas dunglas requested a review from soyuka October 23, 2018 05:53
@dunglas
Copy link
Member

dunglas commented Oct 23, 2018

Let's wait for @soyuka review regarding the Doctrine part.

@soyuka
Copy link
Member

soyuka commented Oct 23, 2018

We should ask @Simperfit instead it's his code and I'm also not sure to what it fixes 😆

@antograssiot
Copy link
Contributor Author

at least if we need to keep it I'll update the tests because even it is tested, dropping this code doesn't make the test suite failing...

@soyuka
Copy link
Member

soyuka commented Oct 23, 2018

I don't think that these are useful and they just look like copy/paste to me (that said I'd be totally in favor of dropping the lines)

@antograssiot
Copy link
Contributor Author

antograssiot commented Oct 23, 2018

ok this was needed as long as we supported doctrine/orm 2.2 so basically means api-platform 1.x
i'll drop it and the associated tests that were not very reliable anyway.

@dunglas dunglas merged commit 069abf2 into api-platform:2.3 Oct 23, 2018
@dunglas
Copy link
Member

dunglas commented Oct 23, 2018

Thanks @antograssiot!

@antograssiot
Copy link
Contributor Author

@dunglas @soyuka I've a working branch based on 2.3 passing PHPStan level 6, is it something you're interested in or would it make community contributions harder?
2.3...antograssiot:phpstan-level6

@dunglas
Copy link
Member

dunglas commented Oct 24, 2018

It's something we're definitely interested in!

@teohhanhui
Copy link
Contributor

teohhanhui commented Oct 24, 2018

@soyuka:

We should ask @Simperfit instead it's his code and I'm also not sure to what it fixes 😆

Are you sure it was not my code? 😝

(I think the commit history was lost when we moved to this repository...)

@antograssiot
Copy link
Contributor Author

@teohhanhui the PR targeted 2.3 (which is stable to me) and will land into master when 2.3 will get merged inside or did I miss something ?

@teohhanhui
Copy link
Contributor

Oops... Sorry, I must have misread. 🙈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants