controller improved #2304
controller improved #2304
Conversation
|
Looks OK. |
|
Done, ready for review. Should we rename |
| $this->stdout(" Fixture \"{$fixture::className()}\" was successfully loaded. \n", Console::FG_GREEN); | ||
| } | ||
|
|
||
| $this->loadFixtures($this->createFixtures($fixtures)); |
There was a problem hiding this comment.
The only reason for the code duplication is to display the success message after every successful fixture loading.
There was a problem hiding this comment.
yes, i noted this. i think we can skip it, because it is better to show if all fixtures are loaded, since we are using transaction, to avoid confusing user, so he will not be doing extra checks to be sure that fixtures was not accidentally applied.
There was a problem hiding this comment.
should we change apply/clear actions to load/unload for more consistency?
There was a problem hiding this comment.
I don't think we should add --noDb.
Perhaps add something like --globalFixtures which takes an array of fixture class names. By default, it is InitDbFixture.
There was a problem hiding this comment.
Perhaps add something like --globalFixtures which takes an array of fixture class names.
sounds good. but you need to add array feature for options like it is done for method params.
There was a problem hiding this comment.
so if option is set to '' empty string, array will be empty like [] correct?
Some code cleanup, related with my comments: