chore: merge upstream/main (baf14b9, unique operationIds) - #1
Merged
Merged
Conversation
Signed-off-by: Shane Loretz <sloretz@intrinsic.ai>
Brings google/gnostic baf14b9 "fix: operationId must be unique (google#478)" into the fork: the 2nd and later http rules of a method now get an "_N" suffix so operationIds stay unique (OpenAPI v3.0.3 4.7.10.1). Conflict resolution in cmd/protoc-gen-openapi/generator/generator.go: the fork's @sse feature (7922890) added an `isSSE` argument to buildOperationV3, while baf14b9 switched the same call site to the new `opID` variable. Both changes are kept - the call site now passes `opID` together with `isSSE`. `fmt` was already imported on the fork side. additional_bindings/openapi.yaml auto-merged: the fork's infoURL change and upstream's `Messaging_UpdateMessage_2` change are on different lines.
Follow-up to the upstream merge: baf14b9 makes the 2nd and later http rules of a method emit operationIds with an "_N" suffix, but only upstream's own openapi.yaml fixture was updated. The fork additionally checks in the fq_schema_naming / json / string_enum / default_response variants for every test dir, and plugin_test.go diffs every fixture that exists, so those four still encoded the old duplicate id and failed. Regenerated with GENERATE_FIXTURES=true, then set back to false (TestGenerateFixturesIsFalse guards the flag). The diff is exactly 4 files, 1 line each, matching upstream's own fixture change.
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.
Why
Fork sync: merge
google/gnosticbaf14b9("fix: operationId must be unique (google#478)")into
main.mainwas 10 commits ahead / 1 behind the merge-basee0e09f7, so this is a real mergecommit — a fast-forward, the web "Sync fork" button and a plain
git pullcannot do it.Upstream's change: the 2nd and later
httprules of a method now emitoperationId: <op>_<N>so operationIds stay unique (OpenAPI v3.0.3 4.7.10.1).Conflict resolution
cmd/protoc-gen-openapi/generator/generator.go(1 hunk): the@ssefeature (7922890)added an
isSSEargument tobuildOperationV3, whilebaf14b9switched the very samecall site to the new
opIDvariable. Both changes are kept:.../additional_bindings/openapi.yamlauto-merged: theinfoURLline and upstream'sMessaging_UpdateMessage_2line are far apart.Fixture follow-up (2nd commit)
baf14b9updates upstream's ownadditional_bindings/openapi.yaml, but this repo alsochecks in the
fq_schema_naming/json/string_enum/default_responsevariants forevery test dir, and
plugin_test.godiffs every fixture that exists. Those four stillencoded the duplicate operationId and failed. Regenerated via
GENERATE_FIXTURES=true(4 files, 1 line each) — identical to upstream's own fixture change.
Verification
The four previously failing subtests (
TestOpenAPIFQSchemaNaming,TestOpenAPIJSONNaming,TestOpenAPIStringEnums,TestOpenAPIDefaultResponse— allAdditional_Bindings) pass.Net diff against
mainis exactly 6 files and contains nothing beyond upstream's own patchplus those 4 fixture lines: no fork customization was lost.
This branch carries a real merge commit (
a08ae5a) whose second parent isbaf14b9.Squash-merging would drop
baf14b9frommain's ancestry, leaving the merge-base ate0e09f7— every futuregit merge upstream/mainwould then re-deliver that commit andre-examine everything since
e0e09f7. Merge commit also matches how this repo syncedupstream before (
562c644).