Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix external reference propagation #1203

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ShouheiNishi
Copy link
Contributor

Fix for #1202

@bartsmykla
Copy link

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

@jamietanna
Copy link
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
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
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
Member

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

@ShouheiNishi
Copy link
Contributor Author

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

Yes.

@bartsmykla
Copy link

@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

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

@ShouheiNishi
Copy link
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
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
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.

None yet

4 participants