Document the write diagnostics and VR mismatch in the README. - #73
Merged
Merged
Conversation
v0.28.0 added two diagnostics the README never mentioned. WriteOptions did not appear in it at all, so the write-side hook -- the headline of the release -- was discoverable only from the changelog or godoc. The read section still described the 0.27.0 set of anomalies and said diagnostics carry their "enclosing sequences", which stopped being the whole truth when Diagnostic.Path started naming which item of each. The write example is built on SetInt past the int32 an IS allows rather than the fractional float the changelog leads with, because SetFloat rejects a float for an IS at the call site: an example written that way could never reach the hook it was demonstrating. Verified each construction against the real API before choosing -- an over-long DS string reports, and SetFloat(SliceThickness, 1/3) now reports nothing at all, since 0.28.0 truncates it per FormatNumberAsDS. The section closes on that boundary, because it is the thing a reader will otherwise learn by accident: the dictionary-VR setters reject what they can see themselves, and the hook covers what they cannot.
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.
v0.28.0 shipped two diagnostics the README never mentioned.
WriteOptionsdid not appear in the README at all, so the write-side hook — theheadline of #67 — was discoverable only from the changelog or godoc. The
read-side section still listed the 0.27.0 set of anomalies, with no VR
disagreement (#68), and said a diagnostic carries its "enclosing sequences",
which stopped being the whole truth when
Diagnostic.Pathstarted naming whichitem of each (#69). Adding the README paragraph in the same commit as the
feature is what #54 did for read diagnostics; I missed it three times in a row.
The example changed shape because the obvious one is impossible
The changelog leads the write diagnostics with a fractional
float64in anIS, so that is what I first wrote the README example on. It cannot happenthrough the typed API:
SetFloatrejects it at the call site, so an example written that way wouldnever reach the hook it was demonstrating. I checked each construction against
the real API before picking one:
SetInt(EchoNumbers, 3000000000)is outside [-2147483648, 2147483647]SetString(SliceThickness, "0.3333333333333333")is 18 bytes, over the 16 a DS allowsSet(NewDataElement(EchoNumbers, VRIS, 1.5))"1.5" is not an integer stringSetFloat(EchoNumbers, 1.5)SetFloat(SliceThickness, 1.0/3.0)FormatNumberAsDSSo the example uses
SetInt, the comment under it is the diagnostic's realoutput byte for byte, and the section closes on the boundary that last row
implies: the dictionary-VR setters reject what they can see themselves, and the
hook covers what they cannot.
No behaviour change —
README.mdand aDocs:changelog entry underUnreleased, following the v0.26.0 and v0.24.0 precedent for doc-only entries.
The three reported cases above are already pinned by
write_diagnostic_test.go,so I did not add a duplicate test; the repo has no
Examplefunctions orREADME-snippet harness, and introducing one is a bigger call than this fix.