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.

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

Add correct return type to DependentFixtureInterface::getDependencies #332

Merged
merged 1 commit into from Jan 11, 2020

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Dec 28, 2019

This makes it easier for PHPStan to understand what's going on.

@alcaeus
Copy link
Member

alcaeus commented Dec 28, 2019

How does PhpStorm handle the class-string type? I’m worried that it may not understand what’s going on...

@ruudk
Copy link
Contributor Author

ruudk commented Dec 28, 2019

I checked. It just sees it as array of mixed, and that was already the case :)

Screenshot 2019-12-28 at 11 48 42

@ruudk
Copy link
Contributor Author

ruudk commented Dec 28, 2019

I'm sure PHPStorm will soon support all the new PHPStan/Psalm notations.

@alcaeus
Copy link
Member

alcaeus commented Dec 28, 2019

Great. I’ll take care of this after the holidays, as I need to check the branches and do some more work here for the persistence deprecations.

I believe this should be fixed in a patch release, would you agree?

@ruudk
Copy link
Contributor Author

ruudk commented Dec 28, 2019

Thanks! I agree.

@greg0ire

This comment has been minimized.

@ruudk ruudk changed the base branch from master to 1.4.x December 31, 2019 16:14
@ruudk

This comment has been minimized.

@greg0ire greg0ire closed this Jan 4, 2020
@greg0ire greg0ire reopened this Jan 4, 2020
@greg0ire

This comment has been minimized.

@greg0ire greg0ire requested a review from a team January 7, 2020 21:40
@ruudk
Copy link
Contributor Author

ruudk commented Jan 9, 2020

@greg0ire Can we merge it? :)

@greg0ire
Copy link
Member

greg0ire commented Jan 9, 2020

I'm not sure: 1.3 is still maintained, isn't it affected by this too?

@ruudk ruudk changed the base branch from 1.4.x to 1.3.x January 9, 2020 19:51
@ruudk
Copy link
Contributor Author

ruudk commented Jan 9, 2020

@greg0ire Changed base branch and rebased it :shipit:

@greg0ire
Copy link
Member

greg0ire commented Jan 9, 2020

Aaaaand the test fail… please cherry-pick b7532e4, that should fix it.

@ruudk
Copy link
Contributor Author

ruudk commented Jan 9, 2020

Sorry, but that's not part of this PR. Clearly 1.3.x- branch is broken. I tried to cherry pick it but that forces me to fix a merge conflict. This such a small change, that I already spend too much time on.

@greg0ire
Copy link
Member

greg0ire commented Jan 9, 2020

Ok don't worry, I'm going to take care of fixing the branch, just leave this open.

@greg0ire
Copy link
Member

greg0ire commented Jan 11, 2020

Ok so apparently it is no longer maintained (see #335), so targeting 1.4.x was the right thing to do, sorry.

@greg0ire greg0ire changed the base branch from 1.3.x to 1.4.x January 11, 2020 11:50
# Conflicts:
#	lib/Doctrine/Common/DataFixtures/DependentFixtureInterface.php
@greg0ire
Copy link
Member

Thanks @ruudk !

@alcaeus alcaeus added this to the 1.4.1 milestone Jan 13, 2020
@alcaeus alcaeus added the Bug label Jan 13, 2020
@alcaeus alcaeus removed the request for review from a team January 13, 2020 07:45
@vitek-rostislav
Copy link

Hi, this made my phpstan in version 0.11.6 fail on

invalid type Doctrine\Common\DataFixtures\class

😞

Luckily, upgrading to ^0.12 version helped.

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

Successfully merging this pull request may close these issues.

None yet

5 participants