Fixed migration type hint#7369
Fixed migration type hint#7369splitt3r wants to merge 2 commits intonextcloud:masterfrom splitt3r:fix-migration-type-hint
Conversation
| /** | ||
| * @param IOutput $output | ||
| * @param \Closure $schemaClosure The `\Closure` returns a `Schema` | ||
| * @param \Closure $schemaClosure The `\Closure` returns a `SchemaWrapper` |
There was a problem hiding this comment.
As far as I can see in https://github.com/nextcloud/server/blob/master/lib/private/DB/MigrationService.php#L392-L407 this is actually a Schema
There was a problem hiding this comment.
That´s right. Will update it.
There was a problem hiding this comment.
May be a stupid qustion but why do the pre and post methods get the Schema and the change method get the SchemaWrapper? This way i would need to add Prefix to the table names in the pre and post methods but can directly use the table name without prefix in the change method. Will look quite strange? Are there any reasons for that?
There was a problem hiding this comment.
It is a good question. I don't know. Once @nickvergessen is back maybe he can awnser
There was a problem hiding this comment.
I think in the beginning the schema wrapper was only for the delete operations.
Now it also does the auto-prefix etc. so hmm would make sense. But I fear we can't do it anymore since that would break the API, so we now have to make sure that SchemaWrapper implements Schema or we have to introduce new interfaces....
Sad enough Schema is not our class, so we can't add the name-magic in there.
There was a problem hiding this comment.
But the API isn´t documented public yet? Still no chance to change it?
There was a problem hiding this comment.
We checked all apps that are in the app store, there are only 2 and I'm the maintainer of both 🎉
So I fixed all the stuff in a new PR: #7909
| /** | ||
| * @param IOutput $output | ||
| * @param \Closure $schemaClosure The `\Closure` returns a `Schema` | ||
| * @param \Closure $schemaClosure The `\Closure` returns a `SchemaWrapper` |
The type hint seems to be wrong. It´s an instance of the Nc SchemaWrapper and not the Doctrine Schema. The SchemaWrapper has some differences and PHP 7 complains about the wrong type once type hint is added.
See: https://github.com/nextcloud/server/blob/master/lib/private/DB/MigrationService.php#L397
All in all the migrations are really nice 💪