GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||
GeezerVison is showing github.com — fix: ignore kin-openapi source locations when comparing type definitions by eyoshioka · Pull Request #2601 · oapi-codegen/oapi-codegen Conversation
Contributor
Fixes #2599. Since #2465, the loader sets This implements option 2 from my comment on the issue. It compares the schemas the same way as Tests:
I used Claude to help write this change. I've reviewed and tested it myself.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Since oapi-codegen#2465 the loader sets IncludeOrigin, so every schema records the line it was declared on. TypeDefinitionsEquivalent compared schemas with reflect.DeepEqual, so two identical schemas with the same x-go-type-name on different lines no longer matched, and generation failed with "duplicate typename". Compare the schemas like reflect.DeepEqual does, but treat any two *openapi3.Origin values as equal. Comparing the JSON encodings instead would drop resolved $ref targets and OpenAPI 3.1 $ref siblings, so different schemas could be merged silently. Fixes oapi-codegen#2599 Co-Authored-By: Claude Opus 5.5 <[email protected]>
eyoshioka
marked this pull request as ready for review
|
There was an error while loading. Please reload this page.
[Medium impact] The PR appears safe to merge; no new actionable issues were found. SummaryUpdates
Reviews (3) · Last reviewed commit: "Revert "fix: blank origins and reuse ref..." · Reviewed by Greptile |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…valent Instead of carrying a copy of reflect's deepValueEqual that skips *openapi3.Origin, walk both schemas once, nil every reachable Origin field, run the real reflect.DeepEqual, and restore the origins afterwards. Origins are still read later (for example to order paths by source line), so a test checks that they come back. Co-Authored-By: Claude Fable 5.1 <[email protected]>
…gin-in-type-equivalence
|
Thanks for doing this. I'm going to push a change to it. Rather than reimplementing so much of DeepEquals, we're going to back up and zero out the Origin on the schemas, then restore before exiting, so that we can reuse the existing DeepEquals representation. It's not as clean, but it's a lot simpler and works for us. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
NVM, what I did is worse. Apologies. I'll put it back as it was, and I merged main into your branch so this applies cleanly. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…ionsEquivalent" This reverts commit a4d1d606c1cc5bd5e1dbe42f2a2d7af1fa3c9bd4. Clearing the origins before calling reflect.DeepEqual was a mistake. Origins sit on every nested Schema, SchemaRef and Discriminator, so the "clear and restore" walker ends up being its own recursive traversal with cycle detection, while also mutating shared schema objects during the comparison. The original read-only comparer that skips *openapi3.Origin is the simpler and safer approach, so this restores it as submitted. Co-Authored-By: Claude Fable 5.1 <[email protected]>
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
v2.8.0: identical schemas with the same x-go-type-name now fail with "duplicate typename"
There was an error while loading. Please reload this page.