ci: run gofmt, vet, build, and tests on pull requests - #43
Merged
Conversation
The only workflow was release-please on push to canary, so nothing verified a branch before it merged and unformatted files reached canary unnoticed. Also reformats internal/ir/auth.go and internal/ir/operations.go, which the new gofmt step flags.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The repo had no CI.
.github/workflows/held onlyrelease-please.yml, which triggers on push tocanary— nothing rango build,go test,gofmt, orgo vetagainst a branch before it merged. That's howinternal/ir/auth.goandinternal/ir/operations.goreachedcanaryunformatted.What this adds
.github/workflows/ci.yml, onpull_requestand on push tocanary:gofmt -l ., failing with a diff if anything is unformattedgo build ./...go vet ./...go test ./...go.mod(go-version-file), so the toolchain tracks the module instead of drifting from a pinned string.concurrencycancels superseded runs on force-push.permissions: contents: read— the job needs nothing else.The test step is the load-bearing one:
internal/generator's e2e tests generate clients from the testdata specs and then compile and run them, so a broken template fails in CI rather than in a consumer's build. The generated packages have no external dependencies, so those nestedgo testruns need no network.Also in this PR
gofmt -won the two offending files — pure comment realignment inAuthSchemeandOperationDef, no semantic change. Without it the new gofmt step fails on arrival.Still needed (org-level, not something a PR can set)
The
parallelworksorg ruleset applied to this repo's default branch requires 2 approvals, linear history, and squash merges — but has norequired_status_checksrule. So even once this workflow exists, a red run won't block a merge. Addingci / buildas a required check on~DEFAULT_BRANCHwould close that gap.