Skip to content

Switch from glog to klog#2787

Merged
AlCutter merged 5 commits intogoogle:masterfrom
jdolitsky:switch-to-klog
Aug 18, 2022
Merged

Switch from glog to klog#2787
AlCutter merged 5 commits intogoogle:masterfrom
jdolitsky:switch-to-klog

Conversation

@jdolitsky
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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

@mhutchinson
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

/gcbrun

@AlCutter AlCutter assigned AlCutter and unassigned mhutchinson Aug 16, 2022
@AlCutter
Copy link
Copy Markdown
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 AlCutter, hickford 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
Copy Markdown
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