Skip to content

util/tracing/otlptracegrpc: remove errNoClient, and use consistent alias#6494

Open
thaJeztah wants to merge 2 commits intomoby:masterfrom
thaJeztah:errs_alias
Open

util/tracing/otlptracegrpc: remove errNoClient, and use consistent alias#6494
thaJeztah wants to merge 2 commits intomoby:masterfrom
thaJeztah:errs_alias

Conversation

@thaJeztah
Copy link
Member

util/tracing/otlptracegrpc: remove errNoClient

This error was not exported, and didn't appear to be used as a sentinel
error. It was added in 750f9af. In a later
refactor (f57b3ef), it was moved to a separate
file, as it appears to be intentional to use a stdlib error, instead of an
error from pkg/errors. Moving it iniline makes it slightly more clear that
it's intentionally using stdlib, but also adding a comment, just in case.

This error was not exported, and didn't appear to be used as a sentinel
error. It was added in 750f9af. In a later
refactor (f57b3ef), it was moved to a separate
file, as it appears to be intentional to use a stdlib error, instead of an
error from `pkg/errors`. Moving it iniline makes it slightly more clear that
it's intentionally using stdlib, but also adding a comment, just in case.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Comment on lines -83 to +85
return errNoClient
// Intentionally using a stdlib error here; see https://github.com/moby/buildkit/commit/f57b3ef89b3053048e7f46d0296d98bcef79448b
return stderrors.New("no client")
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @tonistiigi in case it was not required to use stdlib errors for this; if so, we can drop the extra import and just use errors.New() from pkg/errors.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, just leave pkg/errors to get stacktrace.

@thaJeztah
Copy link
Member Author

Ah, heh; that's why I had it as "draft" locally; forgot that there were quite some un-aliased imports and the linter currently enforces using an alias;

 > [golangci-lint 1/1] RUN --mount=target=/go/src/github.com/moby/buildkit     --mount=target=/root/.cache,type=cache,id=lint-cache-default-linux/arm64   xx-go --wrap &&   golangci-lint run --build-tags "" &&   touch /golangci-lint.done:
76.14 	"errors"
76.14 	^

Unfortunately that linter doesn't allow enforcing for specific imports, only "all" imports it knows about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants