-
Notifications
You must be signed in to change notification settings - Fork 226
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
Move error handling for kpt live commands into kpt error resolver #1840
Conversation
In general, can you include a sample output in the PR descriptions for UX changes. |
This doesn't really make any changes to the output, it just consolidates the mechanism we use to resolve errors to user-facing messages. An example of what the output looks like if applying a set of resources times out:
|
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.
Looks good to me. Minor comments related to naming. Will defer to @frankfarzan for any output message related review.
remove an extra blank line here
use a blank line to improve readability.
|
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.
nits on UI
6ae8751
to
ed99234
Compare
ed99234
to
ed2310e
Compare
…tdev#1840) * Move error handling for kpt live commands into kpt error resolver * Addressed comments
This combines the new error resolver in kpt with the similar solution in cli-utils. With this change, we use one way to handle errors from both git and live.
This also expands the ErrorResolver somewhat by letting the resolver also determine the exit code for an error.