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

ci: enforce all files are gofmt'd. #194

Closed
wants to merge 7 commits into from
Closed

Conversation

cpu
Copy link
Member

@cpu cpu commented Jul 11, 2019

Additionally run all existing files through gofmt -s -w to ensure CI will pass with the new test.

@cpu
Copy link
Member Author

cpu commented Jul 11, 2019

The Travis CI build failed

Blech. The go fmt in CI isn't respecting the vendor deps and generating output. Will iterate in a moment.

@cpu
Copy link
Member Author

cpu commented Jul 11, 2019

Almost there. CI is passing now but seems to have run two jobs in a matrix where I wanted one job with both env vars set.

@cpu
Copy link
Member Author

cpu commented Jul 11, 2019

Getting this to work with the vendored dependencies is trickier than expected... This issue indicates it should work. I thought my trouble was due to process substitution being whacky but I know the GOFLAGS I added directly to the go fmt are being processed because my typo in b44c9c9 broke the go fmt, but the correct value in ef2668c isn't being respected so I'm a bit stumped 🤔

I'm going to try cloning outside of my GOPATH and working on a fix locally instead of spamming the CI queue with my flailing 😓 I'll reopen this PR shortly.

@cpu cpu closed this Jul 11, 2019
@cpu
Copy link
Member Author

cpu commented Jul 11, 2019

Re-opened in a working state in #197

In a bit of cosmic irony we had already bumped into this exact problem in Boulder and arrived at the work-around I used in 195:
https://github.com/letsencrypt/boulder/blob/74699486ec3338c99a633459fa553c586366f6ca/test.sh#L72-L74

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