Skip to content
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

Feat: More explicative error messages #42

Merged
merged 8 commits into from
Jan 21, 2024
Merged

Conversation

ENDERZOMBI102
Copy link
Member

Makes most CLI error messages also display the VTFLib error that caused them, making understanding what happens way easier.

Closes #41

@ENDERZOMBI102 ENDERZOMBI102 requested a review from JJL772 January 20, 2024 01:23
Copy link
Member

@JJL772 JJL772 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple small comments. Looks great otherwise though!

src/cli/action_convert.cpp Outdated Show resolved Hide resolved
src/cli/action_convert.cpp Outdated Show resolved Hide resolved
Review proposed "No error", but is misleading, as this fuction is only called where we know there was an error
As pointed out by review, using `,` is incositent with the rest of the messages, which all use `:`
@ENDERZOMBI102 ENDERZOMBI102 self-assigned this Jan 20, 2024
@ENDERZOMBI102 ENDERZOMBI102 added the enhancement New feature or request label Jan 20, 2024
@ENDERZOMBI102 ENDERZOMBI102 requested a review from JJL772 January 20, 2024 02:58
Copy link
Member

@JJL772 JJL772 left a 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. Thanks!

@ENDERZOMBI102 ENDERZOMBI102 requested a review from JJL772 January 21, 2024 00:07
@ENDERZOMBI102 ENDERZOMBI102 marked this pull request as ready for review January 21, 2024 00:07
@JJL772 JJL772 merged commit 9f6da3a into main Jan 21, 2024
3 checks passed
@JJL772 JJL772 deleted the feat/better-error-messages branch January 21, 2024 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error messages are vague
2 participants