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

build: make TestVet report vet errors #9442

Merged
merged 1 commit into from
Sep 18, 2016

Conversation

yaojingguo
Copy link
Contributor

@yaojingguo yaojingguo commented Sep 17, 2016

Fixes #9441


This change is Reviewable

@@ -107,7 +107,7 @@ TestReturnCheck() {
}

TestVet() {
! go tool vet -all -shadow -printfuncs Info:1,Infof:1,InfofDepth:2,Warning:1,Warningf:1,WarningfDepth:2,Error:1,Errorf:1,ErrorfDepth:2,Fatal:1,Fatalf:1,FatalfDepth:2,UnimplementedWithIssueErrorf:1 . 2>&1 | \
Copy link
Contributor

Choose a reason for hiding this comment

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

can you do something like this instead

vet=$(go tool vet -all -shadow -printfuncs Info:1,Infof:1,InfofDepth:2,Warning:1,Warningf:1,WarningfDepth:2,Error:1,Errorf:1,ErrorfDepth:2,Fatal:1,Fatalf:1,FatalfDepth:2,UnimplementedWithIssueErrorf:1 . 2>&1)
! echo $vet | grep ....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A new commit is submitted. Differences with your suggestion:

  • use local vet in stead of 'vet'.
  • put double quotes around $vet. Otherwise, embedded newlines will be deleted.

@tamird
Copy link
Contributor

tamird commented Sep 18, 2016

LGTM

@yaojingguo
Copy link
Contributor Author

@tamird Could you please help me merge this PR? I have lost the permission to merge PR because of Github two-factor authentication. Now I have enabled Github two-factor authentication. I am waiting for @mjibson to re-add me back.

@tamird
Copy link
Contributor

tamird commented Sep 18, 2016

Sure!

@tamird tamird merged commit 1fb6605 into cockroachdb:master Sep 18, 2016
@yaojingguo yaojingguo deleted the issue-9441 branch May 23, 2017 02:37
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.

2 participants