Add Descriptive Error Message to Upgrade Command #752
Conversation
cmd/upgrade.go
Outdated
|
|
||
| https://exercism.io/faqs`, err) | ||
| } | ||
| return err |
There was a problem hiding this comment.
Seeing as we check for the error above lets return fmt.Errorf(...) within the condition and just return nil here.
There was a problem hiding this comment.
@farisj thanks for your change! The code looks good, just a few nitpicks.
Regarding the rewording of the error message I suggest that you take a look at the error messaging for other commands (e.g download or submit) to get a sense of tone and general formatting.
Happy Hacktoberfest 🎉
cmd/upgrade.go
Outdated
| return updateCLI(c) | ||
| err := updateCLI(c) | ||
| if err != nil { | ||
| err = fmt.Errorf(`We were not able to upgrade the cli because we encountered an error: %s |
There was a problem hiding this comment.
No need to reassign err here. Lets return the error message immediately to fail the command.
This is how the current error message will be displayed to the user.
Error: We were not able to upgrade the cli because we encountered an error: <error message>
Please check the FAQ for solutions to common upgrading issues.
Could you move the message down one line and reword the messaging a bit so that it doesn't contain error:.
Error:
We were not able to upgrade the cli....
<error message>
Please check the FAQ for solutions to common upgrading issues.
https://exercism.io/faqs
Hi there! I'm an exercism user but first time contributor. Happy Hacktoberfest! 😄 🎃
What This Does
Solves #680
This PR adds a wrapper around the
exercism upgradecommand which provides a more human-readable error message along with instructions to visit the FAQ page.Currently, the FAQ page does not yet have upgrade debugging solutions, so perhaps it would be best to wait until exercism/website-copy#417 is merged as well.
Thanks for taking the time to read this PR!