[framework] deprecated MultidomainMigrationTrait.php #2270
Conversation
s3tezsky
force-pushed
the
ds-reliable-db-migrations
branch
2 times, most recently
from
d0a4966
to
3c99cc6
Compare
s3tezsky
changed the title
s3tezsky
force-pushed
the
ds-reliable-db-migrations
branch
from
3c99cc6
to
671d050
Compare
s3tezsky
changed the title
s3tezsky
force-pushed
the
ds-reliable-db-migrations
branch
3 times, most recently
from
eafe5ab
to
db02809
Compare
s3tezsky
force-pushed
the
ds-reliable-db-migrations
branch
from
db02809
to
c84df76
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| /** | ||
| * @return array | ||
| */ | ||
| protected function getParsedDomainConfigs(): array |
There was a problem hiding this comment.
When you have container loaded here - would not be better to use Domain class instead of parsing yaml files?
| /** | ||
| * @return array | ||
| */ | ||
| protected function getCreatedDomainIds(): array |
There was a problem hiding this comment.
I would name it something like getDomainIdsFromSettingValues to be more describing.
| @@ -0,0 +1,72 @@ | |||
| <?php | |||
There was a problem hiding this comment.
I would update commit message to something like:
replaced MultidomainMigrationTrait trait by AbstractMigration class
There was a problem hiding this comment.
Also those two commits should be probably squashed as now the first commit violates YAGNI and use is added in second one.
| /** | ||
| * @return array | ||
| */ | ||
| protected function getAllDomainIds(): array |
There was a problem hiding this comment.
This method violates YAGNI now - it is not used. I thought it will be only method for retrieving domain ids here.
|
Thanks @TomasLudvik for a review but I am closing this PR after discussion we had. As we aggred this change will not introduce such a good change so we rather describe current behavior better. Created #2276 and closing this one. |
domains.yaml