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

Stop using pkg/errors, switch to native golang errors, add errorlint and gofumpt #148

Merged
merged 6 commits into from
Jun 25, 2021

Conversation

kolyshkin
Copy link
Collaborator

@kolyshkin kolyshkin commented Jun 11, 2021

  • Remove all uses of pkg/errors, switching to golang's native error wrapping.
  • Replace obsoleted os.IsPermission / os.IsNotExist with errors.Is
  • Enable errorlint, fix or suppress its warnings
  • Enable gofumpt linter
  • Make sure to not return bare unix errors (errno) from public functions.

@kolyshkin kolyshkin requested review from rhatdan and thaJeztah June 11, 2021 18:24
go-selinux/label/label_linux.go Show resolved Hide resolved
go-selinux/selinux_linux.go Outdated Show resolved Hide resolved
go-selinux/selinux_linux.go Outdated Show resolved Hide resolved
@kolyshkin kolyshkin requested a review from thaJeztah June 15, 2021 17:11
The main benefit of using pkg/errors was the ability to wrap errors.
(The other benefit is saving the error traceback but I doubt any users
of go-selinux rely on that).

However, since Go 1.13 one can use native error wrapping and unwrapping
(fmt.Errorf with %w, errors.Is and errors.As).

Replace all uses of pkg/errors with stdlib. For wrapping errors, use
either %w or, where appropriate, os.PathError.

This change can be disruptive to users who rely on wrapped errors.
They should change to using errors.Is / errors.As to unwrap to check the
error type.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
os.IsNotExist doc says

> New code should use errors.Is(err, os.ErrNotExist).

Same for is.IsPermission:

> New code should use errors.Is(err, os.ErrPermission).

Fix accordingly.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For errors returned from unix pkg, we choose to suppress the warnings,
as unix errors are always bare.

Also suppress error about direct comparison with strconv errors,
as strconv errors are unwrappable in Go 1.13.

Use errors.Is for the remaining case.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
gofumpt is a direct replacement of gofmt with a few extra rules that
make great sense.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Instead of returning a bare errno from lgetxattr, wrap it into
os.PathError to provide more context.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Wrap this into os.PathError to provide some context.

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

@thaJeztah PTAL

Copy link
Collaborator

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

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

LGTM

@rhatdan
Copy link
Collaborator

rhatdan commented Jun 25, 2021

@vrothberg @giuseppe PTAL

Copy link
Collaborator

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@rhatdan rhatdan merged commit e7d5148 into opencontainers:main Jun 25, 2021
@kolyshkin kolyshkin mentioned this pull request Jul 30, 2021
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.

4 participants