Use goimports #86
Use goimports #86
Conversation
.circleci/config.yml
Outdated
| @@ -15,6 +15,10 @@ jobs: | |||
| keys: | |||
| - v1-external-dep | |||
| - v1-dep-{{ checksum "Gopkg.lock" }} | |||
|
|
|||
| - run: test -z "$(goimports -l -w . | tee /dev/stderr)" | |||
| name: Ensure goimports has been run | |||
There was a problem hiding this comment.
The goimports command should be configured using the tool pre-commit in a .pre-commit-config.yaml. You can then check it, and any other pre-commit hooks, in the CI build via:
- run: pre-commit install
- run: pre-commit run --all-filesThere was a problem hiding this comment.
Got it. Did I choose the right place to insert the run, is this where I should add the pre-commit hook?
There was a problem hiding this comment.
Prob better to put it just above run-go-tests. BTW, I'm not sure if Circle has goimports or pre-commit
installed by default...
There was a problem hiding this comment.
I'll check their docs to see if they need to be installed.
There was a problem hiding this comment.
So it looks pretty common to create your own hooks repo, e.g., for you all it could be github.com/gruntwork-io/pre-commit.
Or would you rather use another Github users go-imports hook from pre-commit hooks page.
Also, I didn't see many examples of users running pre-commit hooks via their CI (travis or CircleCI).
There was a problem hiding this comment.
Ha, now that you mention it, we actually had a repo with pre-commit hooks already at Gruntwork, but they were in a private repo. I've extracted them into an open source one: https://github.com/gruntwork-io/pre-commit. There's a go fmt hook there already. Feel free to use that or add a goimports one.
We enforce pre-commit hooks in CI to make sure people are actually running them. The two commands I listed above show how.
There was a problem hiding this comment.
I added the goimports hook to pre-commit repo.
|
This requires gruntwork-io/pre-commit#2 to be merged first. |
.pre-commit-config.yml
Outdated
| @@ -0,0 +1,4 @@ | |||
| - repo: github.com/gruntwork-io/pre-commit | |||
| sha: HEAD | |||
There was a problem hiding this comment.
oops, missed this in the last update. Adding that now
.circleci/config.yml
Outdated
| @@ -29,6 +30,10 @@ jobs: | |||
| paths: | |||
| - $HOME/.go_workspace/src/github.com/$CIRCLE_PROJECT_USERNAME/$CIRCLE_PROJECT_REPONAME/test/vendor | |||
|
|
|||
| - run: pip install pre-commit | |||
There was a problem hiding this comment.
Can you add a comment that says something like, "Fail the build if the pre-commit hooks had not been run."
- Add gruntwork-io/pre-commit goimport hook - Update acronyms to have consistent case in `terraform_http_example_test.go` - https://github.com/golang/go/wiki/CodeReviewComments#initialisms
|
Hm, the build failed with a really odd error: |
|
Oh, I see it. The file has a |
|
oh crap. I noticed that when running it locally and forgot to fix. I can send up a fix if you want, right now |
|
No prob, I just committed the fix :) |
|
BTW, I found this repo because I stumbled upon your talk at HashiConf '17 on YouTube. I really enjoyed it, thanks! |
|
OK, we are now in business! Thanks for adding this.
Ah, I'm very happy to hear that :) Hope you find Terratest useful! |
terraform_http_example_test.go