Skip to content

declare list types #907

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 3 commits into from
Dec 14, 2022
Merged

declare list types #907

merged 3 commits into from
Dec 14, 2022

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Dec 14, 2022

closes #905

@staabm staabm force-pushed the list branch 3 times, most recently from 92f48dd to 90854a3 Compare December 14, 2022 10:43
@staabm staabm marked this pull request as ready for review December 14, 2022 10:49
@staabm
Copy link
Contributor Author

staabm commented Dec 14, 2022

@nikic I think the last commit fixed a bug, which phpstan reported because of the more precise typing.

I am not sure what todo about the unit tests which fail now. could you give me a hand?

nikic added a commit that referenced this pull request Dec 14, 2022
nikic added a commit that referenced this pull request Dec 14, 2022
Found by staabm in #907.

(cherry picked from commit 21a3e8c)
@nikic
Copy link
Owner

nikic commented Dec 14, 2022

Yeah, that was indeed a bug. I've applied the fix separately in 21a3e8c and cherry-picked this into 4.x as well. Can you please rebase?

tools: composer:v2
- name: "Install dependencies"
run: |
composer global require phpstan/phpstan:1.9.3
Copy link
Owner

Choose a reason for hiding this comment

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

Can we install this from tools/composer.json instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@staabm
Copy link
Contributor Author

staabm commented Dec 14, 2022

should be good to go, thx

@nikic nikic merged commit 8ad4129 into nikic:master Dec 14, 2022
@staabm staabm deleted the list branch December 14, 2022 22:05
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.

2 participants