Add $database parameter for methods: listTableIndexes, listTableDetails #4402
Conversation
There was a problem hiding this comment.
Please also add ?string type to the added $database arguments instead of just the phpdocs.
If your commits aren't separated logical steps, please squash your commits into one or either give them a better message instead of using the same commit message for all 4 commits.
| */ | ||
| public function listTableDetails($name) | ||
| public function listTableDetails($name, $database = null): Table |
There was a problem hiding this comment.
Adding a return type in a minor version is a BC break.
| public function listTableDetails($name, $database = null): Table | |
| public function listTableDetails($name, $database = null) |
| @@ -320,9 +320,9 @@ protected function _getPortableTableForeignKeysList($tableForeignKeys) | |||
| /** | |||
| * {@inheritdoc} | |||
| */ | |||
| public function listTableDetails($name) | |||
| public function listTableDetails($name, $database = null): Table | |||
There was a problem hiding this comment.
Adding a return type in a minor version is a BC break.
| public function listTableDetails($name, $database = null): Table | |
| public function listTableDetails($name, $database = null) |
| * | ||
| * @return Index[] | ||
| */ | ||
| public function listTableIndexes($table) | ||
| public function listTableIndexes(string $table, ?string $database = null) |
There was a problem hiding this comment.
This is a BC-break for extending classes, I think you will have to resort to func_get_args if you want to make this BC.
There was a problem hiding this comment.
True, the second argument is a BC break. 😞
| @@ -566,4 +571,136 @@ public function testParseNullCreateOptions(): void | |||
|
|
|||
| self::assertEquals([], $table->getOption('create_options')); | |||
| } | |||
|
|
|||
| public function testListTableIndexes2Db(): void | |||
There was a problem hiding this comment.
This has to be tested with all platforms, not only MySQL which most likely will not work. There is some related work in progress: 3.0.x...morozov:issues/3826.
See the "Testing Different Database Platforms" section drafted in #4356.
Summary
Rebase pull request: #885