You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Do this last: earlier issues delete or move a large share of the text this one would otherwise edit.
Why
44% of the lines in generator/src/main are comments (6151 of 14132). Ten files are more comment
than code:
File
Comment share
SqlIdentifiers.kt
71% (161 of 226 lines)
SqlStarItem.kt
64%
SqlOutputClause.kt
60%
NodeTreeProvenanceExpression.kt
57%
NodeTreeNullabilityAnalyzer.kt
56%
SqlLexer.kt
55%
PgNodeExpression.kt
55%
Model.kt
54%
NodeTreeProvenance.kt
53%
ColumnNullabilityAnalyzer.kt
52%
Most of the mass is not contract. It is argument: why an alternative was rejected, which other
function already handles a case, what a previous bug looked like, and cross-references to other KDoc
("see X's KDoc for why"). Those paragraphs go stale the moment the referenced code moves, and a reader
who wants the contract has to find it inside the essay.
The rule
Each KDoc block keeps at most three things:
The contract. What the function accepts, what it returns, what null/empty means. One to
three sentences. Every optional or nullable parameter says what null means (project rule).
The counterexample. When the contract encodes a non-obvious PostgreSQL fact, one concrete SQL
snippet that shows it, in backticks, with the server version it was checked on. Example, replacing
14 lines on foldAsciiCase: "PostgreSQL folds only ASCII A-Z: on 18.4, with a column "ü", SELECT Ü FROM t fails with column "Ü" does not exist." A counterexample stays true as long as
PostgreSQL does, and a reader can re-run it.
The invariant the caller must uphold, when there is one ("rawIdentifier must be the source
text with quotes intact").
Everything else is cut, not condensed:
"Why not X" paragraphs. If the rejected alternative produced a wrong answer, that wrong answer is
the counterexample (rule 2); the narrative around it goes.
Pointers to other KDoc or to test names ("see FooTest's bar case"). These are the fastest to
rot. The counterexample carries the fact instead.
Restatements of what the code visibly does.
Descriptions of a previous bug ("before this fix...", "this replaced a pg_depend name-join...").
Git history holds those.
Inline // comments that repeat a KDoc paragraph from another function ("See analyzeNodeTree's
identical guard").
Procedure
Work one file at a time, largest comment share first. For each KDoc block:
Read the block and the code it documents.
Write the contract sentence(s). If you cannot state the contract in three sentences, the function
is doing too much; note it and move on, do not paper over it with prose.
Scan the old text for live-verified PostgreSQL facts (anything that quotes an error message, a
version, or a result). For each, decide: is it the fact the contract rests on? Keep it as a
counterexample. Is it a supporting anecdote? Cut it. Before cutting a fact, grep generator/src/test for its SQL or error text; if no test pins it and the fact is load-bearing,
keep it as the counterexample rather than cutting it.
Delete every cross-reference sentence whose only content is "see X".
Do not change any code. Do not rename anything. Do not reformat surviving sentences beyond what spotlessApply requires.
What "done" looks like, per file
Targets, not hard limits; a file that lands above target with every block passing the rule is fine,
and a file that hits the target by deleting a load-bearing counterexample is not.
≤ 45% (dense semantics; every safe-list rule keeps its counterexample)
PgNodeExpression.kt, Model.kt
≤ 45% (@property lines are contract)
NeverNullSafeLists.kt
leave as is; its per-entry audit bullets are the counterexamples
Measure with:
awk '{t++; s=$0; sub(/^[ \t]+/,"",s); if (s ~ /^(\*|\/\*\*|\/\*|\*\/|\/\/)/) c++} END{printf "%d%%\n", c*100/t}' FILE
Test design
No behavior changes, so no new tests. The check that nothing load-bearing was lost is step 3 of the
procedure: a fact is cut only if a test already pins it or it is not what the contract rests on.
Acceptance criteria
git diff --stat shows changes only inside comments (git diff -w --ignore-blank-lines on each
file, with comment lines stripped, is empty). Concretely: ./gradlew :generator:compileKotlin
produces byte-identical class files before and after, or the reviewer spot-checks that every hunk
is inside a /** */ or // block.
Every file in the table is at or under its target, or the PR description names the file and the
counterexample(s) that justified staying above it.
No KDoc in generator/src/main contains the phrase "see .* KDoc" or references a test class by
name (git grep -n -E "Test\]|Test's|see .*KDoc" generator/src/main returns nothing).
./gradlew :generator:check passes.
Files
Every file in the table above.
Conventions every issue inherits
Repo root: /Volumes/Code/3rd-party/norm. Module under change is almost always generator/.
Style: 2-space indent, 120 columns, full words in identifiers (parameter, not param), no
section-separator comments, @Nested classes group tests. Format with ./gradlew spotlessApply.
TDD: tests are written first or alongside. For a behavior-preserving refactor the existing suite is
the pin; each issue says which tests must also be added.
Golden files under test-scenarios*/ are never hand-edited. A refactor is behavior-preserving only
if ./gradlew :gradle-plugin:generateGoldenFiles leaves git status --porcelain test-scenarios test-scenarios-frameworks empty.
Verification for a generator/ change: ./gradlew :generator:check :gradle-plugin:test (Docker
required). Then the golden regeneration check above.
Commit message explains the design decision, not the diff.
Never run ./gradlew clean or disable the configuration/build cache to "fix" a build problem.
Do this last: earlier issues delete or move a large share of the text this one would otherwise edit.
Why
44% of the lines in
generator/src/mainare comments (6151 of 14132). Ten files are more commentthan code:
SqlIdentifiers.ktSqlStarItem.ktSqlOutputClause.ktNodeTreeProvenanceExpression.ktNodeTreeNullabilityAnalyzer.ktSqlLexer.ktPgNodeExpression.ktModel.ktNodeTreeProvenance.ktColumnNullabilityAnalyzer.ktMost of the mass is not contract. It is argument: why an alternative was rejected, which other
function already handles a case, what a previous bug looked like, and cross-references to other KDoc
("see X's KDoc for why"). Those paragraphs go stale the moment the referenced code moves, and a reader
who wants the contract has to find it inside the essay.
The rule
Each KDoc block keeps at most three things:
null/empty means. One tothree sentences. Every optional or nullable parameter says what
nullmeans (project rule).snippet that shows it, in backticks, with the server version it was checked on. Example, replacing
14 lines on
foldAsciiCase: "PostgreSQL folds only ASCIIA-Z: on 18.4, with a column"ü",SELECT Ü FROM tfails withcolumn "Ü" does not exist." A counterexample stays true as long asPostgreSQL does, and a reader can re-run it.
rawIdentifiermust be the sourcetext with quotes intact").
Everything else is cut, not condensed:
the counterexample (rule 2); the narrative around it goes.
FooTest'sbarcase"). These are the fastest torot. The counterexample carries the fact instead.
pg_dependname-join...").Git history holds those.
//comments that repeat a KDoc paragraph from another function ("See analyzeNodeTree'sidentical guard").
Procedure
Work one file at a time, largest comment share first. For each KDoc block:
is doing too much; note it and move on, do not paper over it with prose.
version, or a result). For each, decide: is it the fact the contract rests on? Keep it as a
counterexample. Is it a supporting anecdote? Cut it. Before cutting a fact, grep
generator/src/testfor its SQL or error text; if no test pins it and the fact is load-bearing,keep it as the counterexample rather than cutting it.
PgNodeExpression.kt:263names a non-existentSqlUtils.kt(Remove model fields nothing populates or reads #273 may already have fixed it).
Do not change any code. Do not rename anything. Do not reformat surviving sentences beyond what
spotlessApplyrequires.What "done" looks like, per file
Targets, not hard limits; a file that lands above target with every block passing the rule is fine,
and a file that hits the target by deleting a load-bearing counterexample is not.
SqlIdentifiers.ktSqlStarItem.ktSqlOutputClause.ktSqlLexer.ktNodeTreeProvenanceExpression.kt,NodeTreeProvenance.ktNodeTreeNullabilityAnalyzer.kt,ColumnNullabilityAnalyzer.ktPgNodeExpression.kt,Model.kt@propertylines are contract)NeverNullSafeLists.ktMeasure with:
awk '{t++; s=$0; sub(/^[ \t]+/,"",s); if (s ~ /^(\*|\/\*\*|\/\*|\*\/|\/\/)/) c++} END{printf "%d%%\n", c*100/t}' FILETest design
No behavior changes, so no new tests. The check that nothing load-bearing was lost is step 3 of the
procedure: a fact is cut only if a test already pins it or it is not what the contract rests on.
Acceptance criteria
git diff --statshows changes only inside comments (git diff -w --ignore-blank-lineson eachfile, with comment lines stripped, is empty). Concretely:
./gradlew :generator:compileKotlinproduces byte-identical class files before and after, or the reviewer spot-checks that every hunk
is inside a
/** */or//block.counterexample(s) that justified staying above it.
generator/src/maincontains the phrase "see .* KDoc" or references a test class byname (
git grep -n -E "Test\]|Test's|see .*KDoc" generator/src/mainreturns nothing)../gradlew :generator:checkpasses.Files
Every file in the table above.
Conventions every issue inherits
/Volumes/Code/3rd-party/norm. Module under change is almost alwaysgenerator/.parameter, notparam), nosection-separator comments,
@Nestedclasses group tests. Format with./gradlew spotlessApply.the pin; each issue says which tests must also be added.
test-scenarios*/are never hand-edited. A refactor is behavior-preserving onlyif
./gradlew :gradle-plugin:generateGoldenFilesleavesgit status --porcelain test-scenarios test-scenarios-frameworksempty.generator/change:./gradlew :generator:check :gradle-plugin:test(Dockerrequired). Then the golden regeneration check above.
./gradlew cleanor disable the configuration/build cache to "fix" a build problem.