Skip to content
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

Split flags by categories in --help #23

Merged
merged 2 commits into from
Nov 18, 2022
Merged

Split flags by categories in --help #23

merged 2 commits into from
Nov 18, 2022

Conversation

feedmeapples
Copy link
Contributor

@feedmeapples feedmeapples commented Nov 18, 2022

What was changed

Splits all flags in --help output by categories:

  • Main (ie. command specific options)
  • Client (--namespace, --address, etc)
  • Formatting
  • Pagination

Why?

structuring --help

Checklist

  1. Closes

  2. How was this tested:

Added a new test TestFlagCategory_IsSet that verifies that all flags have category set. Categories need to be set as otherwise flags will not appear in --help

manually ran:

 workflow list -h                                                                                                                                                                                     ✔ 
NAME:
   temporal workflow list - List Workflow Executions based on a Query

USAGE:
   temporal workflow list [command options] [arguments...]

OPTIONS:
   Client Options:

   --address value                  host:port for Temporal frontend service [$TEMPORAL_CLI_ADDRESS]
   --codec-auth value               Authorization header to set for requests to Codec Server [$TEMPORAL_CLI_CODEC_AUTH]
   --codec-endpoint value           Remote Codec Server Endpoint [$TEMPORAL_CLI_CODEC_ENDPOINT]
   --color value                    when to use color: auto, always, never. (default: "auto")
   --context-timeout value          Optional timeout for context of RPC call in seconds (default: 5) [$TEMPORAL_CONTEXT_TIMEOUT]
   --env value                      Env name to read the client environment variables from [$TEMPORAL_CLI_ADDRESS]
   --namespace value, -n value      Temporal workflow namespace (default: "default") [$TEMPORAL_CLI_NAMESPACE]
   --tls-ca-path value              Path to server CA certificate [$TEMPORAL_CLI_TLS_CA]
   --tls-cert-path value            Path to x509 certificate [$TEMPORAL_CLI_TLS_CERT]
   --tls-disable-host-verification  Disable tls host name verification (tls must be enabled) (default: false) [$TEMPORAL_CLI_TLS_DISABLE_HOST_VERIFICATION]
   --tls-key-path value             Path to private key [$TEMPORAL_CLI_TLS_KEY]
   --tls-server-name value          Override for target server name [$TEMPORAL_CLI_TLS_SERVER_NAME]
   --yes, -y                        Confirm all prompts (default: false)

   Formatting Options:

   --fields value            customize fields to print. Set to 'long' to automatically print more of main fields
   --output value, -o value  format output as: table, json, card. (default: "table")
   --time-format value       format time as: relative, iso, raw. (default: "relative")

   Main Options:

   --archived               List archived Workflow Executions (EXPERIMENTAL) (default: false)
   --query value, -q value  Filter results using SQL like query. See https://docs.temporal.io/docs/tctl/workflow/list#--query for details

   Pagination Options:

   --limit value   number of items to print (default: 0)
   --no-pager, -P  disable interactive pager (default: false)
   --pager value   pager to use: less, more, favoritePager.. [$PAGER]
  1. Any docs updates needed?

@feedmeapples feedmeapples force-pushed the flag-categories branch 2 times, most recently from 090aa4a to 080eeac Compare November 18, 2022 02:02
@@ -43,6 +43,8 @@ import (
"go.uber.org/zap/zapcore"
)

var categoryServer = "Server Options:"
Copy link
Member

Choose a reason for hiding this comment

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

Do you need this category if it’s the only one for this command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I checked the output, it's redundant, I'd remove and exclude this command from the test.

Copy link
Member

@bergundy bergundy left a comment

Choose a reason for hiding this comment

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

Admit to not testing this out to get a feel for the experience but from looking at the code I agree with what you did here

@feedmeapples feedmeapples merged commit 310a0b3 into main Nov 18, 2022
@feedmeapples feedmeapples deleted the flag-categories branch November 21, 2022 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants