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

Fix DB name passing in SqliteSchemaManager::listTableForeignKeys() #6338

Merged
merged 1 commit into from May 3, 2024

Conversation

mvorisek
Copy link
Contributor

Q A
Type bug
Fixed issues n/a

Summary

Fix #5617, '' was changed to 'main', but one occurence was missed.

@mvorisek
Copy link
Contributor Author

mvorisek commented Mar 18, 2024

SqliteSchemaManager::selectForeignKeyColumns currently does not honor DB name, so this fix is relevant only if the SqliteSchemaManager class is overriden.

Is this explanation enough or do you want to a functional test with SqliteSchemaManager class extended for testing this?

@mvorisek
Copy link
Contributor Author

@derrabus may I understand your wishes better?

@derrabus
Copy link
Member

I will always ask for tests. Just don't open PRs without tests and ask if you should write a test. Write the test. Thanks.

@mvorisek mvorisek force-pushed the fix_sqlitemanager_default_db branch 3 times, most recently from 59e6d96 to 2797e73 Compare April 3, 2024 13:28
@mvorisek
Copy link
Contributor Author

mvorisek commented Apr 3, 2024

Is there anything with the CI? SQLite tests and CS/stan tests were passing, I have then rebased the PR on the latest 3.8.x, but now stan/SQLite testing is failing. How to fix it?

@derrabus
Copy link
Member

derrabus commented Apr 3, 2024

I've started a new CI run after your comment and it passed. So no, the CI looks fine to me.

@mvorisek mvorisek force-pushed the fix_sqlitemanager_default_db branch from 2797e73 to 7c0346b Compare April 3, 2024 22:23
@mvorisek mvorisek force-pushed the fix_sqlitemanager_default_db branch from 7c0346b to 95516ba Compare April 5, 2024 14:02
@mvorisek
Copy link
Contributor Author

mvorisek commented Apr 5, 2024

Repushed and all tests are passing. Let me know if there is anything else to address.

@mvorisek
Copy link
Contributor Author

mvorisek commented May 3, 2024

@derrabus can I please have your review?

@derrabus derrabus merged commit bc75742 into doctrine:3.8.x May 3, 2024
92 checks passed
@derrabus
Copy link
Member

derrabus commented May 3, 2024

Thank you.

@mvorisek mvorisek deleted the fix_sqlitemanager_default_db branch May 3, 2024 08:52
@greg0ire greg0ire added this to the 3.8.5 milestone May 3, 2024
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.

None yet

3 participants