-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add summary to worklow watch #251
Conversation
4e5e0bb
to
ea2370b
Compare
trace/summary.go
Outdated
|
||
_, _ = title.Println("Execution summary:") | ||
tb := NewTable(os.Stdout) | ||
tb.AppendRows( |
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.
you should be able to accomplish ~same result with an existing API, ex.
cli/cluster/cluster_commands.go
Line 61 in f8c84ed
return output.PrintItems(c, []interface{}{cluster}, po) |
Have you also considered using it?
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.
Done!
Btw, I found it a bit tricky to implement PrintItems since I didn't find any docs. Am I missing something?
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.
yea it was not documented. Designed this way to accept user's input in a simple form
Let me look into documenting this better
d70beb7
to
47e4055
Compare
pushed few commits to make |
Hi 👋 Thanks |
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 would consider a flag to skip printing the summary but otherwise LGTM.
Thanks.
What was changed
Added workflow execution summary to trace command.
Depends on #233.
Example:
Why?
To provide important information about what workflow execution is traced. This could be extended to include other information as StartTime (or a diff with now).
Checklist
Running temporal (see example provided above)
None