-
Notifications
You must be signed in to change notification settings - Fork 4
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
SshCommand: add option to also connect to tools.db #59
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.
Looks good! Just a small change needed to the option definition and getting.
Command/SshCommand.php
Outdated
@@ -84,6 +90,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int | |||
$username = $input->getArgument('username'); | |||
$service = $input->getOption('service'); | |||
$bindAddress = $input->getOption('bind-address'); | |||
$toolsDb = $input->hasOption('toolsdb'); |
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.
hasOption
just reports whether the option exists, so is always true. If you set it to VALUE_NONE above, and then here use getOption
it'll be a boolean.
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.
Thank you! Now I know how it works :) I made bind-address
have VALUE_REQUIRED
as that's what it should be. Hope it's okay to slip that into the same PR.
This is for convenience for apps that need a tools-db connection Make --bind-address require a value Also add .editorconfig
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.
I know Sam was waiting on this before cutting a release, and the latest commit seems to resolve the outstanding comments, so +2
This is for convenience for apps that need a tools-db connection
Also add .editorconfig