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.

Sign up for GitHub

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

Jump to bottom

testing: go vet submodule compatibility #1168

Merged
merged 6 commits into from
Jan 27, 2020
Merged

testing: go vet submodule compatibility #1168

merged 6 commits into from
Jan 27, 2020

Conversation

grayside
Copy link
Contributor

Fixes #1161

This change takes the following steps:

  • For a given directory, check to see if there are go files not inside a go module. If so, run go vet as normal.
  • For each go module inside the target directory, run go vet. No recursion allowed.

@grayside grayside requested a review from tbpg January 21, 2020 20:49
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 21, 2020
@tbpg
Copy link
Contributor

tbpg commented Jan 21, 2020

Looks like vet failed:

+ for i in $(find $targetDir -name "go.mod")
++ dirname ./memorystore/redis/gae_standard_deployment/go.mod
+ mod=./memorystore/redis/gae_standard_deployment
+ echo 'Running '\''go vet'\'' in ./memorystore/redis/gae_standard_deployment'
+ pushd ./memorystore/redis/gae_standard_deployment
Running 'go vet' in ./memorystore/redis/gae_standard_deployment
+ go vet ./...
/tmpfs/src/github/golang-samples/memorystore/redis/gae_standard_deployment /tmpfs/src/github/golang-samples
go: warning: "./..." matched no packages
no packages to vet

@grayside
Copy link
Contributor Author

Interesting failure state. memorystore/redis/gae_standard_deployment doesn't have any go files, but does have a go.mod.

Is this a bug or a feature that go vet found a problem? Should we delete the go.mod and go.sum file?

@tbpg
Copy link
Contributor

tbpg commented Jan 24, 2020

I think a feature? Seems odd to have a go.mod without any go files. I don't think we need it for the sample.

@grayside
Copy link
Contributor Author

From that standpoint, I'll go ahead and clean up trivial violations in this PR.

@grayside
Copy link
Contributor Author

grayside commented Jan 24, 2020 •
edited
Loading

@ericschmidtatwork this PR is removing an unneeded go.mod and go.sum file from a memorystore app.yaml sample. If you need this code in place let me know.

@grayside
Copy link
Contributor Author

All tests passing. go vet caught another WAI error in run/hello-broken.

tbpg
tbpg approved these changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@tbpg tbpg left a comment

Choose a reason for hiding this comment

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

Thank you!

testing/kokoro/system_tests.sh Outdated
go vet $TARGET

# Remove the golang-only triple-dot suffix.
targetDir="${TARGET%/...}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the convention is all lower case with underscores to separate words.

This was referenced Jan 27, 2020
Closed
Closed
@grayside grayside merged commit 055a88d into master Jan 27, 2020
@grayside grayside deleted the testing/go-vet-modules branch January 27, 2020 18:18
AlisskaPie pushed a commit to MaxxleLLC/golang-samples that referenced this pull request Mar 25, 2020
@grayside @AlisskaPie
testing: go vet submodule compatibility (GoogleCloudPlatform#1168)
276d41f 
* testing: go vet submodule compatibility

* memorystore: remove unneeded go.mod and go.sum

* run/hello-broken: fix broken test

* testing/system_tests.sh: bash style correction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbpg tbpg tbpg approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

testing: go vet fails with nested sub-modules
3 participants
@grayside @tbpg @googlebot