Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1271)

Issue 130490043: code review 130490043: go.tools/cmd/vet: validate calls to t.Log and t.Logf (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by r
Modified:
9 years, 7 months ago
Reviewers:
dsymonds, josharian
CC:
golang-codereviews, josharian, dsymonds
Visibility:
Public.

Description

go.tools/cmd/vet: validate calls to t.Log and t.Logf Be careful not to complain about math.Log and cmplx.Log. Seems worthwhile since t.Log and t.Logf are often written but rarely executed. Nothing new turned up in the standard library. Fixes issue 8504.

Patch Set 1 #

Patch Set 2 : diff -r 8a11e1300048655497f877f9408b0b1ac47886c0 https://code.google.com/p/go.tools #

Patch Set 3 : diff -r 8a11e1300048655497f877f9408b0b1ac47886c0 https://code.google.com/p/go.tools #

Patch Set 4 : diff -r 8a11e1300048655497f877f9408b0b1ac47886c0 https://code.google.com/p/go.tools #

Patch Set 5 : diff -r 1aa68e64738729176cc2b8aa0e6922461a839e22 https://code.google.com/p/go.tools #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -0 lines) Patch
M cmd/vet/doc.go View 1 chunk +1 line, -0 lines 0 comments Download
M cmd/vet/print.go View 1 2 3 3 chunks +14 lines, -0 lines 0 comments Download
M cmd/vet/testdata/print.go View 1 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 6
r
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go.tools
9 years, 7 months ago (2014-08-22 21:59:52 UTC) #1
josharian
Code looks good. Here are the results on my standard corpus. True positives: camlistore.org/third_party/github.com/camlistore/goexif/tiff/tiff_test.go:215: wrong ...
9 years, 7 months ago (2014-08-22 22:42:15 UTC) #2
r
You could use -printfuncs to tell it about your logf (-printfuncs=logf:2), which should override the ...
9 years, 7 months ago (2014-08-22 23:27:14 UTC) #3
josharian
r=dsymonds LGTM but wait for dsymonds, please > You could use -printfuncs to tell it ...
9 years, 7 months ago (2014-08-23 00:00:46 UTC) #4
dsymonds
LGTM This is fine. There's plenty of other places that do something like x.Logf(<something>, "%d", ...
9 years, 7 months ago (2014-08-23 00:06:13 UTC) #5
r
9 years, 7 months ago (2014-08-25 19:31:48 UTC) #6
*** Submitted as
https://code.google.com/p/go/source/detail?r=32d1e9227ea0&repo=tools ***

go.tools/cmd/vet: validate calls to t.Log and t.Logf
Be careful not to complain about math.Log and cmplx.Log.
Seems worthwhile since t.Log and t.Logf are often written but
rarely executed.

Nothing new turned up in the standard library.

Fixes issue 8504.

LGTM=josharian, dsymonds
R=golang-codereviews, josharian, dsymonds
CC=golang-codereviews
https://codereview.appspot.com/130490043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b