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

avoid issues when handling errors #251

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ccoVeille
Copy link
Contributor

  • external errors should be checked with errors.Is
  • external errors should be compared with errors.As
  • errors should be wrapped with %w in fmt.Errorf

@ccoVeille ccoVeille mentioned this pull request Apr 5, 2024
@ccoVeille
Copy link
Contributor Author

This is a draft as I need to check every single changes I made to respect what you said here #249 (comment)

- external errors should be checked with errors.Is
- external errors should be compared with errors.As
- errors should be wrapped with %w in fmt.Errorf
@ccoVeille
Copy link
Contributor Author

Here is a new version where I only used errors.Is/errors.As for external errors.

I followed your recommendation

This is meaningless change applying unthoughtful best practices. The errors are controlled in the package and errors.Is is reasonable only when the error comes from another package, also I don't like for performance reasons. Return statement in else is intentional, not to catch unexpected failure of forgetting return in previous branches, possibly caused by nested branches. The first commit about useless receivers is the only one I can merge in the changes.

@ccoVeille ccoVeille marked this pull request as ready for review April 7, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant