Skip to content

Fix external reference propagation#1203

Closed
ShouheiNishi wants to merge 10 commits intooapi-codegen:mainfrom
ShouheiNishi:fix-ref-propagate
Closed

Fix external reference propagation#1203
ShouheiNishi wants to merge 10 commits intooapi-codegen:mainfrom
ShouheiNishi:fix-ref-propagate

Conversation

@ShouheiNishi
Copy link
Copy Markdown
Contributor

Fix for #1202

@ShouheiNishi ShouheiNishi marked this pull request as ready for review August 23, 2023 06:38
@jamietanna jamietanna modified the milestones: v1.14.0, v1.15.0 Aug 29, 2023
@bartsmykla
Copy link
Copy Markdown

This PR fixes an issue which we also face. @jamietanna what do you think about merging it?

@jamietanna
Copy link
Copy Markdown
Member

Thanks for this - we'll try and get it into the v1.16.0 release, which should be some time in October - things are a bit busy over the next few weeks so not sure how much of my free time I'll be able to give to oapi-codegen - hoping that at some point I can have enough GitHub sponsors to be able to dedicate a full day a week to Open Source!

@jamietanna jamietanna modified the milestones: v1.16.0, v2.1.0 Oct 23, 2023
@jamietanna
Copy link
Copy Markdown
Member

@ShouheiNishi @bartsmykla mind checking if the changes in #1389 have solved this for you?

@jamietanna jamietanna modified the milestones: v2.1.0, v2.2.0 Jan 25, 2024
@ShouheiNishi
Copy link
Copy Markdown
Contributor Author

@ShouheiNishi @bartsmykla mind checking if the changes in #1389 have solved this for you?

No.
We plan to summarize the problems that exist after the fix.

@jamietanna
Copy link
Copy Markdown
Member

To confirm, the fix from #1389 hasn't solved all the external reference issues you're affected by?

@ShouheiNishi
Copy link
Copy Markdown
Contributor Author

To confirm, the fix from #1389 hasn't solved all the external reference issues you're affected by?

Yes.

@bartsmykla
Copy link
Copy Markdown

@ShouheiNishi @bartsmykla mind checking if the changes in #1389 have solved this for you?

Unfortunately I cannot test it now as we reworked our flow to overcome this issue (we are cloning external dependencies and combining them in a one file then referencing int locally), but thank you for looking into this!

@emilien-puget
Copy link
Copy Markdown

What about #1253 that had this issue as a requirement?

@ShouheiNishi
Copy link
Copy Markdown
Contributor Author

What about #1253 that had this issue as a requirement?

PR#1253 adds type conversion. In some of the test codes, the process of getting the destination type does not work correctly due to issue #1202, so some test codes will fail.

@ShouheiNishi
Copy link
Copy Markdown
Contributor Author

To confirm, the fix from #1389 hasn't solved all the external reference issues you're affected by?

At least there are still issues like the test cases in this pull request.

@jamietanna jamietanna removed this from the v2.2.0 milestone Jun 3, 2024
@jamietanna jamietanna added this to the v2.3.0 milestone Jun 3, 2024
@jamietanna jamietanna requested a review from a team as a code owner September 20, 2024 15:09
Jamie Tanna added 3 commits September 20, 2024 16:14
This reverts commit 6077912.
@jamietanna jamietanna modified the milestones: v2.4.0, v2.5.0 Sep 20, 2024
@jamietanna jamietanna modified the milestones: v2.5.0, v2.6.0 May 11, 2025
@mromaszewicz
Copy link
Copy Markdown
Member

Thank you for contributing, and I'm very sorry for taking so long to get to this PR. At this point, the code has changed so much that it's no longer relevant because external ref propagation was fixed in commit ad5eada (Jan 2024). The externalref.go file contains comprehensive functions for ensuring external refs are propagated to all definition types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants