Skip to content

ci: add a PR gate that actually checks the generated code - #548

Merged
Episkey-G merged 1 commit into
masterfrom
ci/add-pr-gate
Aug 18, 2026
Merged

ci: add a PR gate that actually checks the generated code#548
Episkey-G merged 1 commit into
masterfrom
ci/add-pr-gate

Conversation

@Episkey-G

Copy link
Copy Markdown
Collaborator

Why

This repo has had no GitHub Actions at all — only a .travis.yml, and Travis stopped serving open-source repos long ago. So PRs here currently run zero build and zero tests.

That matters for the codegen PRs specifically: mergeable_state: clean on them right now means "nobody is checking", not "it passed".

What this adds

.github/workflows/ci.yml — three checks plus a single ci-gate aggregator, so branch protection only ever needs one required check name no matter how the jobs are rearranged later.

The gate keeps only what decides whether the generated SDK is actually usable:

job what it runs why it's in the gate
ci-syntax go build ./... + gofmtcheck covers services/ (the generated packages). Also catches duplicate imports and duplicate declarations — Go treats both as compile errors
lint go vet ./... real bug patterns (printf arg mismatch, lock copying, unused assignments), not style
test make test-cov

Coverage upload is a continue-on-error step inside test and is deliberately not in ci-gate's needs — an upload credential problem must never turn a PR red.

Also: a test that could not pass reliably

TestClientTimeout pointed the client at https://httpbin.org/delay/2, a third-party public service. When that service is unavailable the assertions fail — measured: it returned 503 and the test failed. There is no testing.Short() guard either, so -short cannot skip it. Test_errorHandler had a second reference to the same host.

Both now use a local httptest server. The handler also selects on r.Context().Done(), so it returns as soon as the client times out and srv.Close() does not block.

.travis.yml is removed — it describes a CI that has not run in years, and leaving it implies coverage that does not exist.

Verified locally

make ci-syntax   exit 0
                 exit 2 on an injected syntax error
                 exit 2 on injected gofmt drift
make lint        exit 0
make test-cov    exit 0   (was failing on TestClientTimeout before this)

Not included

A scheduled compatibility matrix. The declared floor in go.mod is go 1.13 and I have not verified the current tree builds on it — shipping that matrix now would just produce a red nightly. Worth doing as a follow-up, together with deciding whether go 1.13 is still the floor we want to claim.

This repo has had no GitHub Actions at all -- only a .travis.yml, and Travis
stopped serving open-source repos long ago. So PRs here run zero build and zero
tests, which means `mergeable_state: clean` on a codegen PR currently means
"nobody is checking", not "it passed".

What this adds
--------------
`.github/workflows/ci.yml` with three checks and a single `ci-gate` aggregator,
so branch protection only ever needs one required check name regardless of how
the jobs are rearranged later.

The gate keeps only what decides whether the generated SDK is usable:

  ci-syntax   `go build ./...` + gofmtcheck -- covers services/ (the generated
              packages). Also catches duplicate imports and duplicate
              declarations, which Go treats as compile errors.
  lint        `go vet ./...` -- real bug patterns (printf arg mismatch, lock
              copying, unused assignments), not style.
  test        `make test-cov`.

Coverage upload is a continue-on-error step inside `test` and is deliberately
not part of `ci-gate`: an upload credential problem must never turn a PR red.

Also fixes a test that could not pass reliably
----------------------------------------------
`TestClientTimeout` pointed the client at https://httpbin.org/delay/2, a
third-party public service. When that service is unavailable the assertions
fail -- measured: it returned 503 and the test failed. There is no
testing.Short() guard either, so `-short` cannot skip it. `Test_errorHandler`
had a second reference to the same host.

Both now use a local httptest server. The handler also selects on
r.Context().Done() so it returns as soon as the client times out and
srv.Close() does not block.

Removes .travis.yml, which describes a CI that has not run in years.

Verified locally
----------------
  make ci-syntax   exit 0   (and exit 2 on an injected syntax error,
                             and exit 2 on injected gofmt drift)
  make lint        exit 0
  make test-cov    exit 0   (was failing on TestClientTimeout before this)

Not included: a scheduled compatibility matrix. The declared floor in go.mod is
go 1.13 and I have not verified the current tree builds on it, so shipping that
matrix now would just produce a red nightly. Worth doing as a follow-up.
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@Episkey-G
Episkey-G merged commit b7cb35f into master Aug 18, 2026
4 of 5 checks passed
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.

1 participant