Skip to content
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

Update ConsoleRunner's command list to include L2 cache and DBAL reserved keywords commands #6556

Merged
merged 3 commits into from
Jul 22, 2017

Conversation

lcobucci
Copy link
Member

These commands were missing and they're quite important...

@lcobucci lcobucci added this to the 2.6.0 milestone Jul 16, 2017
@lcobucci lcobucci self-assigned this Jul 16, 2017
@lcobucci lcobucci requested a review from Ocramius July 16, 2017 21:49
@Ocramius
Copy link
Member

@lcobucci can we add a test checking the list of registered commands? Or is the console runner untestable?

@lcobucci
Copy link
Member Author

@Ocramius we can add an assertion like (for each command):

self::assertTrue($app->has('dbal:run-sql'));

or:

self::assertTrue($app->has((new \Doctrine\DBAL\Tools\Console\Command\RunSqlCommand())->getName()));

Maybe we can move the list of classes to a constant and reuse that constant to instantiate them and test things, like:

foreach (ConsoleRunner::COMMAND_LIST as $command) {
    self::assertTrue($app->has((new $command())->getName()));
}

What do you prefer?

@Ocramius
Copy link
Member

Ocramius commented Jul 22, 2017 via email

@lcobucci lcobucci changed the title Add L2C commands to the console runner Update ConsoleRunner's command list Jul 22, 2017
@Ocramius
Copy link
Member

LGTM 🚢

@Ocramius Ocramius merged commit 1d8c7f9 into doctrine:master Jul 22, 2017
@Ocramius Ocramius changed the title Update ConsoleRunner's command list Update ConsoleRunner's command list to include L2 cache and DBAL reserved keywords commands Jul 22, 2017
@lcobucci lcobucci deleted the fix/command-list branch July 22, 2017 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants