Skip to content
This repository has been archived by the owner on Aug 3, 2021. It is now read-only.

Fix install:git, make:database, update:env commands #14

Merged
merged 3 commits into from
Jan 14, 2018

Conversation

dexm238
Copy link

@dexm238 dexm238 commented Jan 11, 2018

Add necessary optionMap to some commands

Copy link
Owner

@svenluijten svenluijten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've added some comments/questions as a review 😄

@@ -27,6 +28,7 @@ public function configure()
->addArgument('server', InputArgument::REQUIRED, 'The id of the server to create the database on.')
->addOption('user', null, InputOption::VALUE_REQUIRED, 'The username of an (optional) database user to create.', null)
->addOption('password', null, InputOption::VALUE_REQUIRED, 'The password of the database user. Required with "user".', null)
->addOption('name', null, InputOption::VALUE_REQUIRED, 'The name of the database user.', null)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually the name of the database to create, not the name of the database user 🙂

/**
* @var array
*/
protected $optionMap = [
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required when the keys are the same as the values.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fillData() returns an empty array in this case...

/**
* @var array
*/
protected $optionMap = [
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required when the keys are the same as the values.

@@ -87,7 +87,7 @@ protected function getFileContent(InputInterface $input, $option)
return $filename;
}

if ($filename && ftell(STDIN) === 0) {
if ($filename && ftell(STDIN)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this? ftell() returns the position of the pointer. I'd be happy to use ftell(STDIN) !== false if you were looking for more type-safety here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree)

@svenluijten svenluijten merged commit 3f1fa8a into svenluijten:master Jan 14, 2018
@svenluijten
Copy link
Owner

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants