Skip to content

check the declarations with buf lint and buf breaking - #76

Open
hugowetterberg wants to merge 1 commit into
mainfrom
feature/buf-lint
Open

check the declarations with buf lint and buf breaking#76
hugowetterberg wants to merge 1 commit into
mainfrom
feature/buf-lint

Conversation

@hugowetterberg

Copy link
Copy Markdown
Contributor

Summary

Turns buf's lint and breaking-change rules on for this repository's declarations, without changing a single one of them.

The seven declarations here predate buf's conventions and violate the STANDARD rules in twenty-nine places across nine rules. Every one of them is breaking to fix: a package name and a service name are both in the Connect procedure path and the Twirp path, a message name is in the generated Go that every consumer in the fleet compiles against, and a field name is what Twirp's JSON emits. So nothing is renamed. buf.yaml turns the rules on with no except list and grandfathers the existing files one rule at a time, which means a rule applies to every file that is not listed and a new declaration gets the full set. The block was generated by buf lint --error-format=config-ignore-yaml rather than written by hand, and it only ever shrinks — entries are removed when a file is brought into line, never added.

Breaking changes are checked at FILE rather than WIRE_JSON, because this repository's deliverable is generated Go: renaming a message is wire-compatible and still breaks every consumer that compiles against the package.

The CI job

Three steps, in the order that makes them trustworthy: lint first because it is cheapest and its failures are the most local; then a generation drift check; then the breaking check, last because a stale generated tree would make its result meaningless. The checkout is unshallowed because buf compares against the tip of main.

The drift check is new and is worth having on its own. Nothing in CI regenerated anything before this, so a hand-edited service.pb.go would have gone unnoticed.

Depends on ttab/mage#18

go.mod pins github.com/ttab/mage at a pseudo-version of that pull request's head, since rpc:lint and rpc:breaking do not exist in a released version yet. Re-pin to a released ttab/mage before merging, and re-run mage rpc:generate afterwards to confirm no drift — a mage bump can move the pinned generator versions as well as add targets.

The module-root change in that pull request does not affect this repository: the proto root here is the repository root, so no buf.yaml is generated, the hand-written one is left alone, and generation is byte-identical. Verified on this branch.

Not included

All seven declarations fail buf format --exit-code — a diff of some two thousand lines, dominated by buf moving import below package and unwinding aligned = columns. That belongs in a change of its own, where the regenerated output can be seen to be unaffected.

The seven declarations here predate buf's conventions and violate the STANDARD
rules in twenty-nine places across nine rules. Every one of them is breaking to
fix: a package name and a service name are both in the Connect procedure path
and the Twirp path, a message name is in the generated Go that every consumer
in the fleet compiles against, and a field name is what Twirp's JSON emits. So
nothing is renamed. buf.yaml turns the rules on with no "except" list and
grandfathers the existing files one rule at a time, which means a rule applies
to every file that is not listed and a new declaration gets the full set. The
block was generated by "buf lint --error-format=config-ignore-yaml" rather than
written by hand, and it only ever shrinks.

Breaking changes are checked at FILE rather than WIRE_JSON, because this
repository's deliverable is generated Go: renaming a message is wire-compatible
and still breaks every consumer that compiles against the package.

The CI job runs the lint, then a generation drift check, then the breaking
check. The drift check is new and is the gap worth closing on its own — nothing
in CI regenerated anything, so a hand-edited service.pb.go went unnoticed. It
is ordered before the breaking check because a stale generated tree makes that
result untrustworthy, and the checkout is unshallowed because buf compares
against the tip of main.

The declarations are not reformatted. All seven fail "buf format", a diff of
some two thousand lines, and that belongs in a change of its own where the
regenerated output can be seen to be unaffected.
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