Skip to content

[PropertyInfo] Fix bigint extraction with type info #58371

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 24, 2024
Merged

[PropertyInfo] Fix bigint extraction with type info #58371

merged 1 commit into from
Sep 24, 2024

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Sep 24, 2024

Q A
Branch? 7.1
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

During an upmerge, an issue has been made, indeed:

return [
    new Type(Type::BUILTIN_TYPE_INT, $nullable),
    new Type(Type::BUILTIN_TYPE_STRING, $nullable),
];

must be considered as an union and not a collection (ie: Type::union instead of Type::collection)

@mtarld mtarld requested a review from dunglas as a code owner September 24, 2024 08:12
@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.2" but it seems your PR description refers to branch "7.1".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@carsonbot carsonbot added this to the 7.2 milestone Sep 24, 2024
@mtarld mtarld changed the base branch from 7.2 to 7.1 September 24, 2024 08:12
@mtarld mtarld force-pushed the fix/doctrine-bigint-typeinfo branch from fa764e3 to fa2d16b Compare September 24, 2024 08:18
@mtarld mtarld force-pushed the fix/doctrine-bigint-typeinfo branch from fa2d16b to 4ce5d8e Compare September 24, 2024 08:23
@mtarld mtarld requested a review from stof September 24, 2024 08:24
@alexandre-daubois
Copy link
Member

Is there a way to cover this case with tests?

@mtarld mtarld mentioned this pull request Sep 24, 2024
Merged
@mtarld
Copy link
Contributor Author

mtarld commented Sep 24, 2024

This case was already covered with tests, but tests were wrong as well (see https://github.com/symfony/symfony/pull/58371/files#diff-9c2d5e331a5ea6faf2c82cd0cbccf084d3b38ad9d640a8c0f88dc1ab0975ededL289)

@alexandre-daubois
Copy link
Member

Ah yes, I didn't see that the line came from a test. Nice 👍

@nicolas-grekas nicolas-grekas modified the milestones: 7.2, 7.1 Sep 24, 2024
@nicolas-grekas
Copy link
Member

Thank you @mtarld.

@nicolas-grekas nicolas-grekas merged commit 2d645ef into symfony:7.1 Sep 24, 2024
9 of 10 checks passed
@mtarld mtarld deleted the fix/doctrine-bigint-typeinfo branch September 24, 2024 10:50
@fabpot fabpot mentioned this pull request Oct 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof approved these changes

@xabbuh xabbuh xabbuh approved these changes

@alexandre-daubois alexandre-daubois alexandre-daubois approved these changes

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
Bug PropertyInfo Status: Reviewed
Projects
None yet
Milestone
7.1
Development

Successfully merging this pull request may close these issues.
6 participants
@mtarld @carsonbot @alexandre-daubois @nicolas-grekas @stof @xabbuh