Add repo-infra scripts to verify go source code #167
Conversation
k8s-ci-robot
added
sig/multicluster
hack/verify-go-src.sh
Outdated
| @@ -0,0 +1,111 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
It seems like this script runs all the other scripts. Do we need this?
There was a problem hiding this comment.
It runs all the scripts in hack/go-tools/, which right now is just the two being added in this PR. The exact arguments will be ./hack/verify-go-src.sh -v --rootdir $(pwd), which I can add as a new prow job. So we can have the existing prow job to verify all the generated code, and a new prow job to verify all the go source code. WDYT?
There was a problem hiding this comment.
OK, that seems reasonable to me.
hack/go-tools/verify-govet.sh
Outdated
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| set -o errexit |
There was a problem hiding this comment.
nit: set -euo pipefail is more consistent with the rest of the other scripts.
hack/go-tools/verify-govet.sh
Outdated
| @@ -0,0 +1,20 @@ | |||
| #!/bin/bash | |||
| # Copyright 2017 The Kubernetes Authors. | |||
hack/go-tools/verify-gofmt.sh
Outdated
| set -o nounset | ||
| set -o pipefail | ||
|
|
||
| find_files() { |
There was a problem hiding this comment.
Why can't this use $(go list ./... | grep -v /vendor/) like the vet script below?
There was a problem hiding this comment.
This is because go vet takes a list of packages which is provided by go list, whereas gofmt takes paths to Go source files.
|
@perotinus Updated based on comments and added a section to the development doc on how to run these. PTAL. |
docs/development.md
Outdated
|
|
||
| To verify and fix your Go source files: | ||
|
|
||
| 1. Run `./hack/verify-go-src.sh -v --rootdir $(pwd)` |
There was a problem hiding this comment.
Can you explain where you need to run this from to get pwd to work properly? I assume you have to be in the repo root.
hack/verify-go-src.sh
Outdated
|
|
||
|
|
||
| SILENT=true | ||
| REPO_ROOT=$(dirname "${BASH_SOURCE}")/../.. |
hack/verify-go-src.sh
Outdated
| } | ||
|
|
||
| if ${SILENT} ; then | ||
| echo "Running in the silent mode, run with -v if you want to see script logs." |
|
@font Can you address the comments I made so we can get this in? It would be nice to have it running on PR submissions. |
|
@perotinus Oh sorry, the comments were addressed in my last push on Monday. I should have notified you. |
|
@font Oh, sorry! I should have looked more closely at the patches... /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: font, perotinus 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 |
Adding scripts from repo-infra to help verify go source code. It would be best to vendor in the scripts to execute them directly, but we are blocked on golang/dep#1306.
The first commit adds the scripts and the second commit fixes the errors.
This can be followed up with a prow job.
/sig multicluster