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

[#445] Add gofmt validation to CI #523

Merged
merged 3 commits into from Dec 18, 2019
Merged

Conversation

@pedroMMM
Copy link
Contributor

pedroMMM commented Dec 14, 2019

Add gofmt validation to CI requested on #445

Checklist
  • make test-all (UNIX) passes. CI will also test this.

Description of change

Add a make fmt that checks if the code formating is valid. Ensure it's part of the main make invoked by the CI and fail the build if the formating invalid.

Copy link
Owner

aelsabbahy left a comment

When I ran it locally it failed without providing much info:

  1. Proposed a change to show the info
  2. It fails if you have a vendor folder locally, this won't be the case in CI.. but some local development boxes my have it, in my case it was:
invalid gofmt:
vendor/github.com/achanda/go-sysctl/sysctl.go
vendor/github.com/achanda/go-sysctl/sysctl_test.go
vendor/github.com/aelsabbahy/GOnetstat/Examples/tcp_json.go
vendor/github.com/aelsabbahy/GOnetstat/Examples/udp.go
vendor/github.com/docker/docker/api/types/strslice/strslice_test.go
vendor/github.com/docker/docker/cli/command/image/build.go
vendor/github.com/docker/docker/daemon/logger/splunk/splunk_test.go
vendor/github.com/docker/docker/daemon/oci_windows.go
vendor/github.com/docker/docker/integration-cli/docker_cli_build_test.go
vendor/github.com/docker/docker/libcontainerd/client_windows.go
vendor/github.com/docker/docker/opts/hosts_test.go
vendor/github.com/docker/docker/pkg/authorization/authz_unix_test.go
vendor/github.com/docker/docker/pkg/httputils/httputils_test.go
vendor/github.com/docker/docker/pkg/parsers/kernel/kernel_windows.go
vendor/github.com/docker/docker/reference/store_test.go
vendor/github.com/docker/docker/runconfig/hostconfig_test.go
vendor/github.com/mattn/go-colorable/_example/title/main.go
vendor/github.com/miekg/dns/parse_test.go
vendor/github.com/opencontainers/runc/Godeps/_workspace/src/github.com/pquerna/ffjson/fflib/v1/jsonstring.go
vendor/github.com/patrickmn/go-cache/cache_test.go
vendor/golang.org/x/sys/windows/security_windows.go
make: *** [Makefile:29: fmt] Error 1

Maybe something like:

fmt=$$(gofmt -l $$(find . -path ./vendor -prune -o -name '*.go' -print)) ;

?

Makefile Outdated Show resolved Hide resolved
pedroMMM and others added 2 commits Dec 15, 2019
Co-Authored-By: Ahmed Elsabbahy <aelsabbahy@users.noreply.github.com>
@pedroMMM

This comment has been minimized.

Copy link
Contributor Author

pedroMMM commented Dec 15, 2019

I added both of your suggestions.

@pedroMMM pedroMMM requested a review from aelsabbahy Dec 15, 2019
@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Dec 18, 2019

GO_FILES ignores *_test.go files. Is this intentional?

@pedroMMM

This comment has been minimized.

Copy link
Contributor Author

pedroMMM commented Dec 18, 2019

GO_FILES ignores *_test.go files. Is this intentional?

Yes, analysis tools usually will ignore test files. But with that said, I don't feel strong enough about it so I am open to change it if you want.

@aelsabbahy

This comment has been minimized.

Copy link
Owner

aelsabbahy commented Dec 18, 2019

Ok, cool. Just wanted to make sure it was intentional. Thanks for this (and all the other work so far)!

@aelsabbahy aelsabbahy merged commit 15da66c into aelsabbahy:master Dec 18, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pedroMMM pedroMMM deleted the pedroMMM:issue/445 branch Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.