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

Narrow return types for AbstractQuery::getSingleScalarResult() #10721

Merged
merged 1 commit into from
May 23, 2023

Conversation

phansys
Copy link
Contributor

@phansys phansys commented May 23, 2023

Tests were also improved:

Closes #10695

@phansys phansys force-pushed the query_single_result branch 3 times, most recently from 9855225 to af3071a Compare May 23, 2023 00:59
@phansys phansys marked this pull request as ready for review May 23, 2023 01:07
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

There should be 2 commits if you are doing unrelated things. Also, mixed is technically correct, so the commit about changing the phpdoc should target 2.16.x (while the other commit should target 2.15.x to reduce differences, since it does not touch lib).
I've noticed that scalar is a type that exist for static analysis: https://psalm.dev/docs/annotating_code/type_syntax/scalar_types/#scalar

I'm undecided on whether we should use it or not, I would be OK with any solution.

@phansys
Copy link
Contributor Author

phansys commented May 23, 2023

There should be 2 commits if you are doing unrelated things.

TIL, I never created a PR with commits targeting different branches.

I've noticed that scalar is a type that exist for static analysis

I didn't used scalar in order to keep support for other tools that only recognize native types.

@phansys phansys changed the base branch from 2.15.x to 2.16.x May 23, 2023 08:46
@greg0ire greg0ire merged commit d951aa0 into doctrine:2.16.x May 23, 2023
@greg0ire greg0ire added this to the 2.16.0 milestone May 23, 2023
@greg0ire
Copy link
Member

Thanks @phansys !

@phansys phansys deleted the query_single_result branch May 23, 2023 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants