Repository navigation
ci: run one-function-per-file at upstream's default - #1103
Merged
Merged
Conversation
The ratchet is finished: every scanned root reports zero violations at `max_lines = 1`, so the committed ceiling has nothing left to hold back. Deleting the key hands the rule to upstream's default of one line, which is stricter than any number this repo has carried. `one_function_per_file` was the sole key in each `[python]` / `[typescript]` table, so the bare headers go with it. `[python.coverage]`, `[typescript.coverage]` and `[[python.exempt]]` are separate tables and are unaffected -- verified by running `unit coverage` against a copy of the internals/checks config with the key gone and the floor raised to 101, which fails on the floor it still reads. Rust stays out: upstream ships no default for the rule there, and opting it in is a design call rather than a step on this ratchet. skip-changelog: config-only; `changelog_gate` exempts a package-root testing-conventions.toml by exact match, and the release globs carve it out of every published artifact.
Collaborator
Author
|
Post-merge note on the tool version, for the record: npm |
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.
Fixes #1020.
The last rung. Every scanned root is already clean at
max_lines = 1, so thecommitted ceiling holds nothing back and the key comes out. Upstream's default
for python and typescript is 1, spelled out in the CLI's own help:
Deleting the key therefore makes the gate stricter, not looser. Nothing is
added to replace it — the rule stays in the gate list of every lane that names
one, and stays in the reusable workflow's default set for the two SDK lanes.
Census — all five roots, threshold 1
Measured on
mainat3a1e427in a throwaway clone under/tmp. Each root'sconfig was copied out and
max_linesset to1in the copy; no config inthe repo was edited to take the measurement.
internals/checks/srcinternals/checks/testing-conventions.tomlpackages/ts/srcpackages/ts/testing-conventions.tomlpackages/python/dirsqlpackages/python/testing-conventions.tomlplugins/dirsql-plugin-embeddings/srcplugins/dirsql-plugin-embeddings/testing-conventions.tomlinternals/distcheck/srctesting-conventions.toml(that job passes noconfig:)Tool version. Measured with
testing-conventions@latest, which resolved to0.0.115 — npm
latestmoved from0.0.111mid-campaign, and CI is unpinned,so
latestis what CI will run. Re-measured under0.0.111as well: identicalverdicts, exit 0 on all five. Nothing is pinned or floored anywhere as a result.
Positive controls, because exit 0 alone cannot tell "clean" from "scanned
nothing." A module with two multi-line module-scope functions was planted
inside a copy of each of the five real trees and the same command re-run.
All five failed, each naming the planted
beta:Only the control was named — the rest of each tree stayed clean, so the zeros
above are real scans of real files.
What is deleted, and what is not
one_function_per_filewas the sole key in each[python]/[typescript]table, so the bare header goes with it. Six tables across five files:
testing-conventions.toml[python]testing-conventions.toml[typescript]packages/ts/testing-conventions.toml[typescript]packages/python/testing-conventions.toml[python]internals/checks/testing-conventions.toml[python]plugins/dirsql-plugin-embeddings/testing-conventions.toml[python]Untouched:
[python.coverage],[typescript.coverage],[rust]/[rust.coverage],[e2e], and both[[python.exempt]]entries. Those areseparate TOML tables and need no bare parent header. Root
[rust]stays as is —upstream ships no default for this rule on Rust, which is deliberately out of
scope for the ratchet.
Verified, not assumed. Two things were checked rather than reasoned about:
unit coverageagainst a copy of the post-deletion
internals/checksconfig with the floorraised to an impossible
101:error: coverage check failed — coverage 100.00% is below the required 101%.The table is read; only the one-function-per-file threshold changed.
five edited configs re-runs the census at exit 0, and the two positive
controls still fail against them — the same verdicts as the explicit
max_lines = 1.Verification
just preflight: 0 failing pair(s), 3 skipped — 40 (root, gate) pairs across 8 scan roots, includingone-function-per-fileon all five roots with the deleted-key configs. The three skips arepackaging, which needs a built artifact.deleted-key form), plus the ten control runs above.
E2E Verification
N/A - config-only. No source file changes, so no receipt is owed:
e2e verifyruns
--scope packages/python/dirsql --extra-scope packages/rust/src --extra-scope packages/python/src(and the ts equivalent), and apackages/<pkg>/testing-conventions.tomledit falls in none of them. Confirmedgreen by
just preflight, which runse2e-verifyfor all four attestingpackages against
--base origin/main.Changelog / Migrations
changelog_gatescopes topackages/andplugins/, andis_exemptwaives a package-roottesting-conventions.tomlby exactre.fullmatch(rf"{p}/testing-conventions\.toml", path)— still present onmain. The root andinternals/files are outside its roots entirely. Askip-changelog:line rides the commit as a backstop.PARITY.mdchange: no SDK surface is touched.Release impact
None. Every package's release
globscarve the file out explicitly(
packages/ts/!(testing-conventions.toml)), which is what #944 exists toguarantee, so merging this publishes nothing.
Campaign close-out
mainvia refactor(checks): one function per file across attestation_guard and platforms_mirror #1102.exist on
mainwith identical bodies, andtypescript_table.py/gate.pyare byte-identical to
main. Only docstring wording differed. The branch wasalso
CONFLICTINGand well behind.