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
fix: better error messages and failed execution info #877
Conversation
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.
lgtm
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.
found one suspicious statement
) | ||
|
||
return err | ||
ui.Writer = writer |
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.
Is this thread safe? Write is a global var
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.
No, added comment to rewrite whole ui, to be able to get new instance with writer passed / I know this sucks ;) as this global is good for simple use cases - we can rethink this whole solution on some meeting with team
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.
) | ||
|
||
return err | ||
ui.Writer = writer |
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.
this ui
is ugly I know ;)
) | ||
|
||
return err | ||
ui.Writer = writer |
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.
No, added comment to rewrite whole ui, to be able to get new instance with writer passed / I know this sucks ;) as this global is good for simple use cases - we can rethink this whole solution on some meeting with team
) | ||
|
||
return err | ||
ui.Writer = writer |
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.
Pull request description
Fixing more edge cases in jobs execution
Checklist (choose whats happened)
Breaking changes
Changes
Fixes