- Notifications
You must be signed in to change notification settings - Fork928
feat: add verbose error messaging#3053
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Other than thecheckVersions function that could do with a cleanup, this looks great!
The only thing I noticed isn't touched by this PR, so not exactly relevant, but documenting it anyhow:
We probably shouldn't output the version check error when we can't reach the API as it adds verbosity to the output:
| _, _ = fmt.Fprint(&output, helpErrMsg) | ||
| return cliui.Styles.Error.Render(output.String()) | ||
| } | ||
| func checkVersions(cmd *cobra.Command, client *codersdk.Client) error { |
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.
Happened to notice this function still uses hard-coded version ofvarNoVersionCheck and also has a suppression check (which we're doing in the pre-run as well), do we want to keep both?
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.
Good catch, we don'tneed to keep both checks but I kept the check at the top of the the pre-run to avoid the unnecessary I/O of instantiating a client that we immediately throw away when someone passes the disable-warning flag.
Great point I'll open up another PR to clean up some of that noise |
Uh oh!
There was an error while loading.Please reload this page.
loginfatally errored when checking versions.Regular output:
Verbose output: