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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from May 3, 2024
Merged

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

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 •
edited

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
derrabus added a commit that referenced this pull request May 16, 2024
@derrabus
Merge branch '4.1.x' into 5.0.x
a96c52c 
* 4.1.x:
  Fix example for QB delete and update in doc block
  Bump doctrine/.github from 5.0.0 to 5.0.1 (#6391)
  PHPStan 1.10.67, PHPUnit 9.6.19, PHPCS 3.9.2 (#6387)
  Fix "Plugin 'mysql_native_password' is not loaded" (#6388)
  Run tests against MySQL 8.4 (#6386)
  Use 3.8.x as a target for dependabot version updates (#6384)
  Setup dependabot
  Fix SQLiteSchemaManagerTest case
  Switch to substr implementation
  fix merge
  Bump workflow actions
  Fix SQL Server "extended property" SQL generation (#6353)
  Fix DB name passing in SqliteSchemaManager::listTableForeignKeys() (#6338)
  Fix SQLite temp table name must not contain dot (#6315)
  Provide CODECOV_TOKEN
  Upgrade to codecov/codecov-action v4
  Set fail_ci_if_error flag to true
  Specify the minor version number
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
Bug Missing Tests SQLite
Projects
None yet
Milestone
3.8.5
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mvorisek @derrabus @greg0ire