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

golang1.21 + slog #198

Merged
merged 3 commits into from
Nov 29, 2023
Merged

golang1.21 + slog #198

merged 3 commits into from
Nov 29, 2023

Conversation

thepwagner
Copy link
Contributor

log/slog is pretty good.

For all the logging we do, let's migrate to the stdlib and trim some dependencies.

@thepwagner thepwagner self-assigned this Nov 29, 2023
@thepwagner thepwagner requested a review from a team as a code owner November 29, 2023 14:28
Copy link

@jacobmichels jacobmichels left a comment

Choose a reason for hiding this comment

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

A welcome addition to the stdlib!

@@ -2,8 +2,6 @@ linters:
disable-all: true
enable:
- bodyclose
- deadcode
- depguard

Choose a reason for hiding this comment

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

Why disable some linting?

Copy link
Contributor Author

@thepwagner thepwagner Nov 29, 2023

Choose a reason for hiding this comment

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

Sorry - I missed that in the PR description!

3/4, because they're deprecated and we're using the "replaced by X" linter already. This removes a warning on startup.

WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.

The weirder one was depguard:

internal/cli/app.go:4:2: import 'github.com/go-logr/logr' is not allowed from list 'Main' (depguard)
	"github.com/go-logr/logr"
	^
internal/cli/app.go:5:2: import 'github.com/urfave/cli/v2' is not allowed from list 'Main' (depguard)
	"github.com/urfave/cli/v2"
	^
internal/cli/generate.go:10:2: import 'github.com/go-logr/logr' is not allowed from list 'Main' (depguard)
	"github.com/go-logr/logr"
	^
internal/cli/generate.go:11:2: import 'github.com/goreleaser/nfpm/v2' is not allowed from list 'Main' (depguard)
	"github.com/goreleaser/nfpm/v2"
	^

and it seems to repeat that for every imported dependency.

From the error description, I assumed it depguard's configuration had moved from a "denylist" approach where we list the naughty packages we don't want, to an "allowlist" approach where we need to specify the packages we like.
I didn't find golangci/golangci-lint#3906 until you asked, but it aligns with that assumption 😎 . Since we're upgrading from 1.51 to 1.55, we have to clear the hurdles from 1.53.

I figured since we weren't giving depguard a naughty OR a nice list, it wasn't doing much. Yeet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I appreciate the irony that depguard would be useful to say... encode that github.com/rs/zerolog should be avoided and we'd prefer you to use log/slog instead.

@thepwagner thepwagner merged commit df3fa90 into main Nov 29, 2023
4 checks passed
@thepwagner thepwagner deleted the slog-enfogger branch November 29, 2023 15:31
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

2 participants