-
-
Notifications
You must be signed in to change notification settings - Fork 201
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
Added service for loading data fixtures #248
Conversation
# Conflicts: # Command/LoadDataFixturesDoctrineCommand.php
Shouldn't there be some docs? |
} | ||
|
||
$ui->error($message . '.'); | ||
$doctrineFixtureService = new DoctrineFixtureService($this->fixturesLoader, $em); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be injected through the constructor, this is a DIP violation.
$executor->execute($fixtures, $this->isAppend()); | ||
} | ||
|
||
public function getPurgeMode() : int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is only used internally.
return $this; | ||
} | ||
|
||
public function isAppend() : bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is only used internally
I don't think this PR makes sense at this point and how it was done. The main point is to have the purger and executor injectable, not to extract the logic into a dummy service. As I've explained in #88 (comment), this is currently not possible due to how the underlying library works, but I'm hoping to fix that. In the meantime, I'm closing this PR. Thanks for giving it a shot, @mvmaasakkers! |
No description provided.