-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add support for doctrine/dbal 4 version #439
Conversation
4f4f109
to
51597e1
Compare
env: | ||
MYSQL_ROOT_PASSWORD: root | ||
MYSQL_DATABASE: phpcr_tests | ||
options: >- | ||
--health-cmd "mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=5 | ||
steps: | ||
- name: start | ||
run: docker run -d mysql:8.3 --character-set-server=utf8mb4 --collation-server=utf8mb4_unicode_ci |
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.
found this in https://github.com/orgs/community/discussions/25086 but seems to also no longer work.
after trying 10 other things. @derrabus by any chance would you know how i can tell github actions what charset to use? it seems to default to utf8mb3 (man i hate stupid mysql collations mess)
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.
Does it still work for MySQL 5.7 though? I mean, you don't have to upgrade to 8.3 in order to support DBAL 4. You can still do that in a follow-up.
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.
@dbu i am not sure if it solves the probleme, but if i read the example you shared correct, the steps
is maybe here correct?
https://github.com/jackalope/jackalope-doctrine-dbal/pull/440/files#diff-97081780ba6b676e25ca4729b13035dd4d3fd5f010dd76151028ae821707d3cdR69
But ci is not running on my fork, and runs need approvals
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.
if i switch back to old mysql, the default encoding indeed seems to be utf8 (which afaik is fragile and its recommended to use utf8mb4 instead).
i would love to switch to utf8mb4 by default for mysql to reflect best practices. and as 2.x is not yet released, it would be the right moment for such a BC break.
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.
also, dbal 4 seems to incorrectly report latin1 as encoding, when in reality it should be utf8. the PHP 8.0 builds install dbal 3.8.1, the PHP 8.1-3 install dbal 4.
557a26e
to
dafe312
Compare
4bc0eea
to
40bf8d8
Compare
* Update array string type and int type * Fix removed getForUpdateSQL * Fix modifyLimitQuery offset null * Remove unnecessary mocks * Fix defining the max length everywhere * Give existing schemaconfig in it * Upgrade Mysql from 5 to 8 * Use createSchemaManager always * Replace MysqlPlatform check with AbstractMySQLPlatform for MariaDB compatibility
* bump to dbal 3.8 minimum to avoid problems with 'forUpdate()' * while we phpstan with dbal 3, use the old lowercase form for sqlite * forUpdate is not platform aware, check ourselves * make encoding required
40bf8d8
to
2319292
Compare
continue #436
fix #441