docs: cut comments that explain standard Go, and repair detached godoc - #48
Merged
Conversation
The dispatch arms in doResolve and checkCompat each carried a one-line
gloss of the condition immediately below ("Both unions.", "Same kind.",
"Different kinds: check promotion."), and resolveNode restated its own
doc comment at both of the sites that doc describes. None of them says
anything the code does not.
Schema.Canonical, liftFieldLogicalIntoType, tryAssignNamedRef and validateLeaf each had their doc comment stranded above the NEXT declaration, so godoc attached it to the wrong symbol and Canonical -- exported -- rendered with no documentation at all. Move each back and fix the afieldComplexKeys block, which named a variable that does not exist.
The floating "defaultAsInt32 / defaultAsInt64 / defaultAsFloat" block documented no function -- it sat between numericDefault and a run of unrelated helpers -- and both claims that distinguished it from numericDefault own doc were wrong: defaultAsFloat has no string arm, and its int64/int32 arms convert unconditionally rather than rejecting past-mantissa magnitudes. Also drop the second "register early so self-referencing fields..." (the same sentence eight lines up), collapse the applyCustomTypes copy of buildCustomWiring jsonAppliesLogical rationale to the pointer it already carried, and cut the "setCanon=false" opener from a comment sitting on a call that passes true.
The "Memory bounds" note lost its second limit mid-sentence ("bounds the
compressed size read off the wire (default 64 MiB), and" -> next line
starts a new topic), so the paragraph named one of the two limits it
promises. Name the decompressed bound and split the sentence.
The rest is removal: "useful for" framing on NopCloser and
WithSyncMarker, "Resolve codec." / "Apply schema evolution if a reader
schema was provided." above the lines that do exactly that, and the
history clause on WithCodec ownership paragraph -- the rule it states is
what a caller needs, not when it started applying everywhere.
Godoc that told the reader when to reach for an API rather than what it does: CustomType "Use this when you need full control -- for example, to map a custom Go struct to/from an Avro fixed", NewCustomType "For example, use this to decode Avro longs into a domain-specific ID type", SchemaCache "This is useful for Schema Registry integrations", WithCustomType "it exists for discoverability". Each keeps whatever fact it carried (which form reaches records, what NewCustomType infers) and loses the advice. setIface loses its ten-line sample of the inlined check its own prose already describes, and typeFieldMapping ambiguity note loses "Go makes the selector a compile error" -- true of every Go program, and not why the error is deferred to lookup.
…ents Same defect as the schema.go batch: valueWalkLimit, appendAvroFloat32, findUnionBranch and inferType each had their doc stranded above the following declaration, so godoc showed it against marshalEmitLen, float32WireBits, unionTagTier and baseTypeForLogical instead. inferType was left with only its ptrChain parameter note. valueWalkLimit block also said "two orthogonal limits" above three bullets -- now three. getString had two doc comments stacked, opening with the same sentence twice; merged into one.
"json.Number Kind() is reflect.String" was spelled out at four sites -- the union type-name dispatcher, the enum gate, the RFC 3339 probe, and SchemaFor Kind switch -- each re-deriving the same numeric-carrier policy before naming its own consequence. Put the rule on the jsonNumberType declaration every one of them compares against, and leave each site with the part that is local to it.
Comments that named the line under them: "Handle nil / invalid values." above !v.IsValid(), "ASCII fast path." inside the c < utf8.RuneSelf branch, "Save position for backtracking." above savedPos, "Skip key (string), then value.", "Deserialize each element.", "Handle UTF-16 surrogate pairs.", the struct fast-path label in deserRecord, and deserRecordFast's no-depth-bump note, which repeats its own doc comment verbatim two lines up. Three fragments left over from edits: a doubled "Java's / Java's" in the timestamp-nanos rationale, and two schema_canonical parentheticals recording that a MarshalJSON method used to exist. atype loses the note that its untyped string constants work wherever a string is expected.
predicate_census_test.go Q11 counts "&SemanticError{" occurrences per
file against a registered number, and the count includes matches inside
comments -- so deleting this sample as a worked example took reflect.go
from 6 to 5 and redded the guard. Put it back rather than edit the
registry, which is out of scope for a comment round.
Comment and struct-field alignment only, no semantic change. Split out ahead of the test-file consolidation so those commits stay provably move-only and `gofmt -l` is clean throughout.
Cut as "useful for" framing. It is not framing — it answers why anyone would set a sync marker at all, which is not otherwise derivable from a setter whose default is "random". Every call site in this repo is a test, which is what the sentence says.
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
1 participant
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.
A comment pass over non-test source. Comment-only: no code line is touched in
any of the 11 commits, verified mechanically.
The part that actually matters
Schema.Canonicalhad documentation written for it that nobody could see.A blank line between the comment and the func meant godoc ignored it, so
pkg.go.dev showed the signature and nothing else. Fixed, along with eight more
detached comments on unexported functions.
Also repairs a truncated security note in
ocf: the "Memory bounds" paragraphpromised two independent limits, named
WithMaxBlockBytes, and then stoppedmid-sentence — so the decompressed-size bound it was introducing went unnamed.
The cuts
Comments explaining standard Go rather than library behavior: that a nil
pointer panics, that map iteration is randomized, that ambiguous embedded
fields are a compile error, how a typed nil arises. Plus "this is useful for"
framing, worked examples of calling the function the comment sits on, and
clauses restating the sentence above them.
Comments that justify the code next to them were left alone even where they
discuss Go — why a bound is taken in float space before the int conversion,
why the depth limit is 1000, why an
.IsValid()guard exists. Deleting thoseinvites deleting the guard they explain.
Honest scope
Net effect is only -91 comment lines (10,777 -> 10,686), which moves the
comment ratio from 37% to 36.5%. The remaining mass is not miscategorized
comments, it is long ones: 669 blocks longer than 5 lines hold 77% of all
comment lines. Reducing that needs a rule about length per decision, not about
category, and is a separate pass.
Gate
A doc round has no oracle — every test passes whichever comments you cut. So
the gate is mechanical:
prints nothing.
go vetclean,gofmtclean, full suite and-racegreen.