-
Notifications
You must be signed in to change notification settings - Fork 29
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
Refactor tests: isolate test using errors.Unwrap #17
base: master
Are you sure you want to change the base?
Conversation
Isolate TestWrappedError_IsCompatibleWithErrorsUnwrap which uses errors.Unwrap (which appeared with Go 1.13) in a separate test source compiled only for Go 1.13+.
Hello, thank you for the PR! I am no longer a maintainer on this project, so I can't give you an official position. If I understand correctly, you are making this change to support Go versions earlier than go1.13. The Go team only supports the latest two versions (Go1.21, Go1.22), and most libraries follow the same practice. go1.12 is sufficiently old now that I would say moving the code to a separate file is not worth it. |
The project has no Go version mentioned in go.mod, so there is no official stance about the minimum Go version supported. This project still exists to support downstream projects which haven't yet fully embraced the improved error interface from go1.13 and go1.20. Newer code should move away from it. I'm a candidate for being a co-maintainer if necessary (FYI I'm also the current lead maintainer of Testify, if that matters regarding maintaining an old codebase). |
(Note that I'm also supporting the effort to detach go-multierror from errwrap: hashicorp/go-multierror#60) |
@KaushikiAnand Could you review this or find someone who has the authority? |
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
Isolate TestWrappedError_IsCompatibleWithErrorsUnwrap which uses errors.Unwrap (which appeared with Go 1.13) in a separate test source compiled only for Go 1.13+.
Note: other tests that use
%w
withfmt.Errorf
should be isolated too. I will provide those changes once this PR is accepted.