chore(deps): update gcf-go from v1.5.0 to v1.7.1 - #243
Open
blackwell-systems wants to merge 1 commit into
Open
Conversation
Brings oq's gcf-go dependency current. Three correctness fixes shipped since v1.5.0: - v1.6.1: count enforcement (SPEC 13) - decode rejects a declared row count that does not match the rows present, rather than silently truncating. - v1.7.0: int64 numeric domain - numbers outside the int64 domain are handled explicitly instead of silently losing precision. - v1.7.1: quoted-key / array-value round-trip - fixes a decode bug where a '[' inside a quoted key was read as the array-count bracket. go.mod + go.sum only; no code or API change. EncodeGeneric's signature is unchanged, so oq/format.go is untouched.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
📊 Test Coverage ReportCurrent Coverage: Coverage Change: ✅ No change Coverage by Package
📋 Detailed Coverage by Function (click to expand)
Generated by GitHub Actions |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
TristanSpeakEasy
requested changes
Aug 25, 2026
TristanSpeakEasy
left a comment
TristanSpeakEasy
left a comment
Member
There was a problem hiding this comment.
Reviewed the full gcf-go v1.5.0...v1.7.1 range (39 commits), the final module/API diff, and oq's FormatGCF call path.
The dependency itself looks legitimate: its module path and Go 1.21 floor are unchanged, it has no transitive dependencies, no exported API was removed, and the checksum resolves to the expected v1.7.1 tag. The strict count handling, numeric-domain enforcement, quoted-key parsing, and other included fixes are reasonable.
Requesting changes because this does alter supported oq --format gcf output, contrary to the PR description, and there is currently no GCF formatter coverage to make that compatibility decision explicit. Details are inline.
Validation:
- Compared every upstream commit and the downloaded v1.5.0/v1.7.1 source trees.
- Ran isolated old/new
EncodeGenericprobes for representativeoqrow shapes, array values, marker-shaped strings, count output, empty output, and keyed maps. - Confirmed current PR CI is green: 12 successful, 1 skipped, 0 failing.
|
|
||
| require ( | ||
| github.com/blackwell-systems/gcf-go v1.5.0 | ||
| github.com/blackwell-systems/gcf-go v1.7.1 |
Member
There was a problem hiding this comment.
This upgrade has oq-visible output changes that are missing from the PR description. FormatGCF passes array fields as nested []string values. With v1.5.0, a representative tags/scopes value is formatted as a scalar cell such as [pets public]; v1.7.1 recursively normalizes it and emits a real GCF array attachment (^ plus .tags [2]: pets,public). Strings shaped like ^{...} are also newly quoted instead of emitted bare.
These are worthwhile correctness fixes, but they are byte-level and semantic output changes. The stated petstore smoke query does not exercise them because the default operation fields contain no arrays, and oq/format_test.go has no GCF coverage. Please document/accept the compatibility change explicitly and add golden coverage for at least an array-valued projection and a ^{...} string before merging.
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
2 participants
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.
Brings oq's
gcf-godependency current, from v1.5.0 to v1.7.1. Three correctness fixes shipped since v1.5.0.go.mod+go.sumonly; no code or API change.What's in the update
[inside a quoted key was read as the array-count bracket.Compatibility
EncodeGeneric's signature is unchanged, sooq/format.goneeds no edits. v1.7.0 madeEncodeGenericpanic on numbers outside the int64 domain (withEncodeGenericCheckedas the error-returning variant). oq only passes it strings, Goint(int64-bounded), bools, and arrays of those, so that path is unreachable here. If you'd rather be defensive,EncodeGenericCheckedis a drop-in that returns an error instead.Verification
go build ./...clean in both modules (root andcmd/openapi)go test ./oq/...passesgo mod verifyclean; diff isgo.mod+go.sumonly, no other dependency changesoq 'operations' --format gcfon the petstore fixture produces correct outputSummary by cubic
Updates
github.com/blackwell-systems/gcf-gofrom v1.5.0 to v1.7.1 to pull in correctness fixes; onlygo.modandgo.sumchanged. No code or API changes in this repo; our usage should see no behavior change.EncodeGenericpanics on out-of-domain values. We only encode strings, int, bool, and arrays of these, so unaffected.[inside a quoted key could be misread as an array-count bracket; now decodes correctly.go.mod/go.sumin root andcmd/openapi; build and tests pass.Written for commit 0bb5d3b. Summary will update on new commits.