-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
SchemaValidator: Adapting to new DBAL mapping of bigint #11396
Conversation
Have you tried that on a real project? |
=> So this does indeed look like a step forward to me. |
@@ -53,7 +53,7 @@ class SchemaValidator | |||
*/ | |||
private const BUILTIN_TYPES_MAP = [ | |||
AsciiStringType::class => 'string', | |||
BigIntType::class => 'string', | |||
BigIntType::class => 'string|int', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still support DBAL 3 though… maybe we should keep string
when DBAL 3 is installed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but how can the version of DBAL be detected here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could test for the existence of BigIntType::getName()
with a call to method_exists()
. That method does not exist with DBAL 4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, but how can I do this inside this const
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you can, you'd probably have to convert the const to a property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No tests. 🤷🏻♂️
In #11399 I started over (using git CLI ;-) so I'm closing here. |
Closes #11377