AL-797 - Refined help text #1563
Conversation
TeslaDethray
force-pushed
the
change/element_help_text
branch
8 times, most recently
from
ee30070
to
a3ec106
Compare
| * | ||
| * @usage <site>.<env> Lists all backups made of <site>'s <env> environment. | ||
| * @usage <site>.<env> --element=<element> Lists all <element> backups made of <site>'s <env> environment. | ||
| */ | ||
| public function listBackups($site_env, $element = 'all') | ||
| public function listBackups($site_env, array $options = ['element' => 'all',]) |
There was a problem hiding this comment.
This is something we missed: all the other backup commands use element as an option (and rightfully so because it's optional, even here) but backup:list has it as a parameter. I would like to change it to an option.
TeslaDethray
force-pushed
the
change/element_help_text
branch
from
a3ec106
to
adfc97b
Compare
TeslaDethray
force-pushed
the
change/element_help_text
branch
2 times, most recently
from
639dad1
to
0e93dee
Compare
| @@ -37,14 +33,14 @@ class GetCommand extends TerminusCommand implements SiteAwareInterface, RequestA | |||
| * @usage <site>.<env> --to=<path> Saves the most recent backup of any type in <site>'s <env> environment to <path>. | |||
| * @usage <site>.<env> --to=<path> Saves the most recent <element> backup in <site>'s <env> environment to <path>. | |||
| */ | |||
| public function getBackup($site_env, array $options = ['file' => null, 'element' => null, 'to' => null,]) | |||
| public function getBackup($site_env, array $options = ['file' => null, 'element' => 'all', 'to' => null,]) | |||
There was a problem hiding this comment.
Re our convo @alexfornuto By using the 'element' => 'all' here, the underlying software we're using picks it up and adds it to the help output.
TeslaDethray
force-pushed
the
change/element_help_text
branch
from
0e93dee
to
2572963
Compare
TeslaDethray
changed the title
| * @param string $element | ||
| * @return null|string | ||
| */ | ||
| protected function getElement($element) |
There was a problem hiding this comment.
Q: Can I just validate the element option?
|
This looks fine to me although it leaves some weird undefined behavior when element is sent as an arg and an option. If it were me I'd break bc in favor of maintainable and understandable behavior. I get the semver promise argument though so.... +1 |
TeslaDethray
force-pushed
the
change/element_help_text
branch
from
53d322b
to
bdaa45b
Compare
|
I mean for it to always prefer the option, as it is not deprecated. If the option is simply default, it will accept whatever the parameter has in it. I wanted running scripts w/o the option to run smoothly. I'll note this in a comment. |
TeslaDethray
force-pushed
the
change/element_help_text
branch
from
bdaa45b
to
fae7e76
Compare
No description provided.