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

Add MariaDb1010Platform for fetchTableOptionsByTable #6434

Merged
merged 2 commits into from
Jun 19, 2024

Conversation

grooverdan
Copy link
Contributor

Q A
Type improvement
Fixed issues #6361

Summary

As requested: #6425 (comment)

The diverging MariaDB-10.10.1+ implementation of retrieving table options split the implementation so that its now in the AbstractMySQLPlatform and the MariaDB specific implementation in MariaDBPlatform.

On CI - I don't follow the 'useless $sql' error.

The diverging MariaDB-10.10.1+ implementation of retrieving table
options split the implementation so that its now in the
AbstractMySQLPlatform and the MariaDB specific implementation in
MariaDBPlatform.

Fixes #6361.
@derrabus
Copy link
Member

Thank you. I've simplified the signature of fetchTableOptionsByTable() a little because the method didn't really use the information that you passed in.

Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

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

Codecov is highlighting a lot of uncovered code lines. Could you please take a look if the tests really are sufficient?

@derrabus
Copy link
Member

We get a lot of false positives from CodeCov these days unfortunately.

@derrabus derrabus merged commit 955ec99 into doctrine:3.9.x Jun 19, 2024
91 of 92 checks passed
@grooverdan
Copy link
Contributor Author

Thanks for the merge. Appoligies for missing the signature cleanup.

@grooverdan grooverdan deleted the fetchTableOptions_split.x branch June 19, 2024 07:28
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.

4 participants