-
Notifications
You must be signed in to change notification settings - Fork 42
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
EDU-2415: Improve CLI Help content #561
Conversation
Also: * Adds language for `temporal`, `temporal activity`, `temporal activity complete` and `temporal activity fail`. NOTES * I'm a little confused by the inconsistency of punctuation. * How wordy do you want the command-line usage to be? There's already an overwhelming amount of "wall of text". I was thinking more: ``` * `--env` (string) - Active environment name. Default: default. Env: TEMPORAL_ENV. * `--env-file` (string) - Path to environment settings file (defaults to `$HOME/.config/temporalio/temporal.yaml`). * `--log-level` (string-enum) - Log level. Default is "info" for most commands and "warn" for `server start-dev`. Options: debug, info, warn, error, never. Default: info. * `--log-format` (string) - Log format. Options are "text" and "json". Default is "text". * `--output`, `-o` (string-enum) - Non-logging data output format. Options: text, json, jsonl, none. Default: text. * `--time-format` (string-enum) - Time format. Options: relative, iso, raw. Default: relative. * `--color` (string-enum) - Output coloring. Options: always, never, auto. Default: auto. * `--no-json-shorthand-payloads` (bool) - Show all payloads as raw, even if they are JSON. Default: false. ``` over ``` * `--env` (string) - Read additional options from the `ENV` environment. (See "temporal env --help" for more about environments.) Default: default. Env: TEMPORAL_ENV. * `--env-file` (string) - Read/store per-environment configuration in `PATH` (defaults to "$HOME/.config/temporalio/temporal.yaml"). * `--log-level` (string-enum) - Log level. Default is "info" for most commands and "warn" for the development server. Options: debug, info, warn, error, never. Default: info. * `--log-format` (string) - Emit log messages in "text" or "json" format. Default is "text". * `--output`, `-o` (string-enum) - Emit output in the `FMT` format. Does not affect logs; use --log-format instead. Options: text, json, jsonl, none. Default: text. * `--time-format` (string-enum) - Emit timestamps in the `FMT` format. Options: relative, iso, raw. Default: relative. * `--color` (string-enum) - Colorize output. Does not affect logs. Options: always, never, auto. Default: auto. * `--no-json-shorthand-payloads` (bool) - Show all payloads as raw payloads even if they are JSON. ``` ISSUES 1. There's no way to bundle the `temporal` options, which are imported elsewhere. They have to be there and they are distracting, and contribute to overwhelmed users. 2. Even though I set a description for complete and fail, they are not shown cli% ./temporal activity complete Error: required flag(s) "activity-id", "result", "workflow-id" not set Usage: temporal activity complete [flags] Ditto for temporal activity fail.
Proof of concept material is pretty much there before I move forward.
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.
Thanks! I left a bunch of comments but I think it's all pretty minor.
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.
Awesome. Just gotta pull in the latest main
(with the versioning changes I think) and then we're good.
And conflict resolution
Also adds alias coverage to contribution guidance
Also Query quick-ref
(note, my opinions are just general notes about formatting, they are not meant to block others' approval and will defer to others stewarding this project) |
All comments are appreciated and valuable. Thank you. |
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.
Here's what I found in a quick runthrough of each of your changes. Will start a deeper final pass now but probably won't finish until tomorrow.
Also addresses EDU-2651
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.
Functional comments; trying to figure out why some things in temporal workflow update
seem to have changed between main and your branch. Still reviewing but wanted to give you these to start.
Co-authored-by: Josh Berry <josh.berry@temporal.io>
Thanks! Will wait for the rest. Otherwise I'm doing a failover to the "Write short tutorials" region of my workday. |
Co-authored-by: Josh Berry <josh.berry@temporal.io>
Co-authored-by: Josh Berry <josh.berry@temporal.io>
@fairlydurable Here are my changes for you to review, including reverting some unintentional functional changes: 755b2bb#diff-b4c85d71e771b938a0779d1e4113e068c19dd896aac86c04fe1828dd5a15c63f I wrote a small tool to check for unintentional functional changes so I'm pretty confident this will take care of the build issues as well. (Will consider throwing it up in a separate PR once cleaned up.) |
This is up to date as of mid-day 23 July.
Note: This will not compile in its current state.
Ready for Josh's review.