Introduce a ArrayParameterType enum
#5838
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
derrabus
force-pushed
the
improvement/array-parameter-type
branch
from
45d7a61
to
c6bdad1
Compare
SenseException
approved these changes
Dec 29, 2022
| */ | ||
| public const ARRAY_PARAM_OFFSET = 100; |
There was a problem hiding this comment.
I'm fine with this removal because it's internal. How do others see this?
There was a problem hiding this comment.
ORM seems use it #5837 (comment) and if doctrine/dbal and doctrine/orm is not a match it will fail. So for bc reasons when I think it need to be kept if somebody is only updating doctrine/dbal and not doctrine/orm. Else doctrine/dbal 3.6.0 would require to set a conflict to all doctrine/orm versions lower than a supported version that version to avoid that older none compatible orm versino is used, setting requirement on doctrine/orm side would not work as current orm version already supports ^3.6.
There was a problem hiding this comment.
Good point. I'll restore the constant.
greg0ire
approved these changes
Dec 29, 2022
This was referenced Dec 30, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR deprecates the
Connection::PARAM_*_ARRAYconstants in favor of a newArrayParameterTypeclass that I'd like to turn into an enum in 4.0.x. My motivation is that those constants are still arbitrary integers and cannot be typed properly on PHP 8. Array parameters are also the only remaining scenario in which a parameter type is expressed as an integer. Transitioning to an enum would allow us to eliminate integers.Since we already have a
ParameterTypeenum, it makes sense to create an enum for those group of types as well.The PR also adds a test that is going to fail on 4.0.x and thus reproduces #5837.