-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Reintroduce support for multi-db setups #1426
Conversation
This snipet was lost in the 2.12.x => 2.13.x change. Needs to be reintroduced to allow those who use fully qualified db+table names. (Eg, `database.table`)
Technically it wasn't "lost". The change is due to https://github.com/barryvdh/laravel-ide-helper/pull/1349/files If we're going forward with this PR, we're now keeping a very mixed state here and I'm not sure anyone understands the outcome. Original code: $database = null;
if (strpos($table, '.')) {
[$database, $table] = explode('.', $table);
} PR #1349: $database = $model->getConnection()->getName();
…
$columns = $schema->listTableColumns($table, $database); This PR: $database = $model->getConnection()->getName();
…
if (strpos($table, '.')) {
[$database, $table] = explode('.', $table);
} I.e. is the I think my cautious side suggests we revert PR #1349 rather in its entirety. WDYT? |
Any news about this PR? |
I had the same problem and this solution brings back the old function |
I'm not really sure. @mikebouwmans what do you think? Does this work for you? |
Any news about this PR? |
Me too... |
Closes #1487 |
This snipet was lost in the 2.12.x => 2.13.x change. Needs to be reintroduced to allow those who use fully qualified db+table names. (Eg, `database.table`)
This snipet was lost in the 2.12.x => 2.13.x change. Needs to be reintroduced to allow those who use fully qualified db+table names. (Eg,
database.table
)Summary
Type of change
Checklist
composer fix-style