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

Switch to Go native error wrapping #7

Merged
merged 3 commits into from
Jun 15, 2021
Merged

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Jun 14, 2021

  • switch to native error wrapping
  • add go.mod
  • update go versions in travis.yml

Compatibility notes:

  1. ErrSymlinkLoop is removed in favor of wrapping syscall.ELOOP into
    &os.PathError{}; users should use e.g. errors.Is(err, syscall.ELOOP)
    to check for this particular error.

  2. IsNotExist no longer use pkg/errors.Cause but native go error
    unwrapping; it could breaks a use case when it is used for some
    third-party errors wrapped using pkg/errors.Wrap[f].

I was not able to find any users of either feature.

This is a shorter/simpler alternative to #4. The differences are:

  • no intermediate vendoring of pkg/errors;
  • slightly simpler IsNotExist implementation;
  • removing ErrSymlinkLoop, using os.PathError wrapping to return ELOOP.

@thaJeztah @cyphar PTAL

join.go Outdated Show resolved Hide resolved
@kolyshkin kolyshkin force-pushed the native-err branch 2 times, most recently from 371a324 to 105f75d Compare June 14, 2021 19:40
Compatibility notes:

1. ErrSymlinkLoop is removed in favor of wrapping syscall.ELOOP into
   &os.PathError{}; users should use e.g. errors.Is(err, syscall.ELOOP)
   to check for this particular error.

2. IsNotExist now does not use pkg/errors.Cause but native go error
   unwrapping; it could breaks a use case when it is used for some
   third-party errors wrapped using pkg/errors.Wrap[f].

I was not able to find any users of either feature.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Owner

@cyphar cyphar left a comment

Choose a reason for hiding this comment

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

LGTM. 👍 to dropping everything in one go -- thanks for pushing this originally @thaJeztah!

@cyphar cyphar closed this in 09a7b25 Jun 15, 2021
@cyphar cyphar merged commit 09a7b25 into cyphar:master Jun 15, 2021
@thaJeztah
Copy link

Thanks!

@kolyshkin kolyshkin deleted the native-err branch December 2, 2021 16:29
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

3 participants