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

(#470) Localize github.com/pkg/errors #471

Closed
wants to merge 5 commits into from

Conversation

vaguecoder
Copy link

Motivation

Issue #470 reported by @skormos

Changes

Adds new internal package errors with test coverage.

Summary

Localizes following functions (and required types and methods) from github.com/pkg/errors as it is no longer being maintained.

New(message string) error
Wrap(err error, message string) error

Related Issues

Closes #470
Helps in #462

vaguecoder and others added 3 commits September 7, 2022 00:24
Signed-off-by: Bhargav Ravuri <vaguecoder0to.n@gmail.com>
Signed-off-by: Bhargav Ravuri <vaguecoder0to.n@gmail.com>
@rs
Copy link
Owner

rs commented Sep 12, 2022

After this change, as we do not expose the errors.Wrap, how are users supposed to use this feature?

@vaguecoder
Copy link
Author

@rs I totally missed that Wrap is required by users. Can it be included in the same package as MarshalStack? I understand that it means extending pkgerrors layer.

Signed-off-by: Bhargav Ravuri <vaguecoder0to.n@gmail.com>
Signed-off-by: Bhargav Ravuri <vaguecoder0to.n@gmail.com>
@rs
Copy link
Owner

rs commented Sep 13, 2022

Reading the change I'm not really fan of the resulting API. I wonder if we could keep backward compat (when getting a wrapped error) and at the same time, auto-wrap an error when Stack is present and Err is called. This way we do not expose a new API.

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.

Stack: remove github.com/pkg/errors as a dependency
2 participants