Skip to content

chore: remove fmt.Errorf across the codebase in preference of errors.Errorf/Wrap#3305

Merged
tonistiigi merged 6 commits intomoby:masterfrom
jedevc:errors-linting
Nov 23, 2022
Merged

chore: remove fmt.Errorf across the codebase in preference of errors.Errorf/Wrap#3305
tonistiigi merged 6 commits intomoby:masterfrom
jedevc:errors-linting

Conversation

@jedevc
Copy link
Member

@jedevc jedevc commented Nov 23, 2022

In general, we prefer to use github.com/pkg/errors wherever possible, since it provides stacktraces, and we try and enforce this in code-review. However, this is easy to forget (especially for me 😄) - so we should provide a linting rule to ban using fmt.Errorf. This should stop these leaking through in future PRs.

This PR adds a linter rule to forbidigo, to ban this function, requesting that users switch to using the errors package. As part of this, we also need to update all the instances of fmt.Errorf, which was useful to cleanup some cases of code-rot.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
@tonistiigi tonistiigi merged commit 6f13ac6 into moby:master Nov 23, 2022
@jedevc jedevc deleted the errors-linting branch November 23, 2022 18:07
@tonistiigi tonistiigi mentioned this pull request Nov 23, 2022
@AkihiroSuda
Copy link
Member

The upstream https://github.com/pkg/errors is archived, can we fork it?
Maybe as github.com/moby/errors

@thaJeztah
Copy link
Member

The upstream https://github.com/pkg/errors is archived, can we fork it? Maybe as github.com/moby/errors

That has come up a few times. I know we still have an interest in using that package over Go's standard wrapping (as we use information we use when crossing boundaries, see containerd/containerd#6937 / containerd/containerd#6937 (comment)).

I know we discussed at some point to see if some of us can take over maintainership of pkg/errors. While forking is an option, it's not ideal (if we do, we should at least make sure that it's considered the "canonical" / endorsed fork, otherwise we may end up with multiple parties maintaining a fork).

In general, the package is basically "feature complete", so I don't expect much maintenance (and we definitely shouldn't change the scope of the package), but I see people get nervous because it's archived.

@davecheney would you be open to discuss options?

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