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
Feature: Allow to import databases different than the default one - db-import command #49
Feature: Allow to import databases different than the default one - db-import command #49
Conversation
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.
A couple of minor changes but overall looks good!
src/App/Commands/DbImportCommand.php
Outdated
@@ -23,27 +24,32 @@ protected function configure() | |||
$this->setName('db-import') | |||
->setDescription('Executes db import command') | |||
->setHelp('Import given db into default database.') | |||
->addArgument('filepath', InputArgument::OPTIONAL, 'File to import'); | |||
->addArgument('filepath', InputArgument::OPTIONAL, 'File to import') | |||
->addOption('dbname', NULL, InputOption::VALUE_OPTIONAL, 'The database in that we want to import the data'); |
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.
The database that you want to import the data in
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.
Thx
src/App/Commands/DbImportCommand.php
Outdated
@@ -23,27 +24,32 @@ protected function configure() | |||
$this->setName('db-import') | |||
->setDescription('Executes db import command') | |||
->setHelp('Import given db into default database.') | |||
->addArgument('filepath', InputArgument::OPTIONAL, 'File to import'); | |||
->addArgument('filepath', InputArgument::OPTIONAL, 'File to import') | |||
->addOption('dbname', NULL, InputOption::VALUE_OPTIONAL, 'The database in that we want to import the data'); |
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.
lowercase null please
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.
Thx
src/App/Commands/DbImportCommand.php
Outdated
$databaseName = $_SERVER['MYSQL_DATABASE']; | ||
if ($optionalDatabaseName = $input->getOption('dbname')) { | ||
$databaseName = $optionalDatabaseName; | ||
} |
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.
$databaseName = $input->getOption('dbname') ?? $_SERVER['MYSQL_DATABASE'];
Please try before making the code change but I think this should work
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.
Thx, you right , that's better
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.
LGTM!
This PR provide a new option to allow users to import databases different than the default one.
Issue Reference
Issue Number: #
What does this Pull Request do?
How can this Pull Request be tested?
Any additional notes/comments?