Make govet check glog.Info/Warning calls #16801
Conversation
openshift-ci-robot
added
the
size/S
|
/lgtm |
openshift-merge-robot
added
the
approved
hack/verify-govet.sh
Outdated
| for test_dir in $(os::util::list_go_src_dirs); do | ||
| if ! result="$(go tool vet -shadow=false "${test_dir}" 2>&1)"; then | ||
| if ! result="$(go tool vet ${govet_args} "${test_dir}" 2>&1)"; then |
There was a problem hiding this comment.
If you're using them once, just inline them here
There was a problem hiding this comment.
personally i like the way it's called out as a var, makes it easier to see the args and update them rather than embedding them in a messy line of bash.
There was a problem hiding this comment.
oops, just pushed a change. i'm fine either way
There was a problem hiding this comment.
no need to churn on it so i guess we'll leave it.
The default set of names covered by govet's checking of Printf-like functions includes Log, Error, and Fatal, but not Info or Warning.
danwinship
force-pushed
the
govet-warningf
branch
from
5a210e7
to
a96d4d6
Compare
openshift-merge-robot
removed
the
lgtm
| @@ -30,7 +30,7 @@ function govet_blacklist_contains() { | |||
| } | |||
|
|
|||
| for test_dir in $(os::util::list_go_src_dirs); do | |||
| if ! result="$(go tool vet -shadow=false "${test_dir}" 2>&1)"; then | |||
| if ! result="$(go tool vet -shadow=false -printfuncs=Info,Infof,Warning,Warningf "${test_dir}" 2>&1)"; then | |||
There was a problem hiding this comment.
i'm going on record as liking it better the way it was.
There was a problem hiding this comment.
And expecting that people understand this this one place the "${args}" are not quoted because we need word splitting for proper parameter passing for the call to go tool vet? No thanks
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, danwinship The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/retest |
|
Automatic merge from submit-queue (batch tested with PRs 15807, 16778, 16801, 16804, 16793). |
The default set of names covered by govet's checking of Printf-like functions includes Log, Error, and Fatal, but not Info or Warning. (Noticed in #16793.)