search

equinix / metal-cli

Official Equinix Metal CLI
https://deploy.equinix.com/labs/metal-cli
MIT License
59 stars 46 forks source link

Improve bool definitions and execution behavior #487

Open displague opened 2 months ago

displague commented 2 months ago

In #485, @stevemar, @ctreatma, and I discussed that while the precedent in metal-cli is for bool's to have --name=<true|false>, there is at least one exception with --app vs --sms for TFA.

We also discussed that in places where --name=false is used, we may not be handling it properly, where false values must be explicitly sent in API calls. These parameters are confusing to specify, and with Cobra, it is easy to miss incorrect handling during code review.

Metal CLI should adopt more obvious patterns for bools, such as named pairs (lock, unlock) or --no- prefixes.

It would be great to get these improvements from upstream project(s), but it can be implemented locally:

Originally posted by @displague in https://github.com/equinix/metal-cli/issues/485#issuecomment-2315403934

waltribeiro commented 2 months ago

Is boolean the decided direction? I wouldn't be against a string like @ctreatma mentioned. I assume something like -x lock and -x unlock which feels intuitive.

Last month, Talos Linux spent almost an hour trying to figure out the iPXE boolean on Equinix Metal (here @ 51:52). However, it mustn't have been deployed as true during deployment, so they had to switch it inside the console.