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 from glog to klog #2787

Merged
merged 5 commits into from
Aug 18, 2022
Merged

Switch from glog to klog #2787

merged 5 commits into from
Aug 18, 2022

Conversation

jdolitsky
Copy link
Contributor

Please see the following explanation of the reasons for klog vs. glog: https://github.com/kubernetes/klog#why-was-klog-created

@jdolitsky
Copy link
Contributor Author

This should pull in latest go-licenses once this is merged: google/go-licenses#138

@mhutchinson
Copy link
Contributor

/gcbrun

I've just rebased this to fix conflicts in the go.mod file

@mhutchinson mhutchinson self-assigned this Aug 2, 2022
@mhutchinson
Copy link
Contributor

This is currently failing presubmit checks because klog.Fatalf is not recognized as an exit condition, and we're getting staticcheck errors from this. This is fixed (dominikh/go-tools#1110) but we'll need to update the libraries. Error:

running golangci-lint
experimental/batchmap/cmd/verify/verify.go:101:6: SA5011(related information): this check suggests that the pointer can be nil (staticcheck)
		if leaf == nil {
		   ^

@mhutchinson
Copy link
Contributor

I've got #2791 to update to the latest linter, but it doesn't fix this problem. https://github.com/dominikh/go-tools/blob/v0.3.3/go/ir/exits.go#L100 - looks like more work is required upstream in the staticcheck library to detect the v2 version of klog.

@AlCutter
Copy link
Member

/gcbrun

@AlCutter AlCutter assigned AlCutter and unassigned mhutchinson Aug 16, 2022
@AlCutter
Copy link
Member

I've rebased/jimmied onto HEAD and fixed a few follow-up bits, @hickford would you mind casting an eye over the tweaks so I'm not approving my own code?

@AlCutter AlCutter requested review from hickford, AlCutter and phbnf and removed request for hickford August 16, 2022 13:48
@AlCutter AlCutter merged commit 307637b into google:master Aug 18, 2022
@AlCutter
Copy link
Member

Added #2801 to track removing the //nolint comments once @mhutchinson's fix to golangci-lint is present in a released version.

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