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

Bug: using x-go-type-name results in missing types #1397

Merged

Conversation

jamesphillpotts-fr
Copy link
Contributor

@jamesphillpotts-fr jamesphillpotts-fr commented Dec 19, 2023

This PR describes an issue with type generation when the x-go-type-name is used. The assigned type name is correctly used on the MyTestRequest but types for the enum type and the sub type that also has the x-go-type-name are missing.

@jamesphillpotts-fr
Copy link
Contributor Author

I've managed to make my fix work with a couple of other tweaks to how schema types are used. All tests pass and all generated code tests remain the same except my new one which is fixed by this change.

@jamesphillpotts-fr
Copy link
Contributor Author

Hi @jamietanna @deepmap-marcinr - if you get a minute, is there any chance of triggering the workflow for this PR so that I can make sure it's review-ready? Thanks :-)

@jamietanna jamietanna self-assigned this Jan 8, 2024
@jamietanna jamietanna self-requested a review January 8, 2024 11:58
@jamietanna jamietanna added the bug Something isn't working label Jan 8, 2024
@jamietanna jamietanna added this to the v2.1.0 milestone Jan 8, 2024
@jamesphillpotts-fr
Copy link
Contributor Author

Thanks @jamietanna, it seems to be ok 😌

@jamietanna
Copy link
Member

Thanks for the changes - I'm not going to merge it just yet (and focus on getting the release out now) but will get this in for the next release, hopefully in a couple of weeks. Thanks!

@jamietanna jamietanna modified the milestones: v2.1.0, v2.2.0 Jan 25, 2024
@jamesphillpotts-fr
Copy link
Contributor Author

@jamietanna are we able to get this merged now?

@jamesphillpotts-fr
Copy link
Contributor Author

@jamietanna are we able to get this merged now?

bump :-)

@jamesphillpotts-fr
Copy link
Contributor Author

Hi @jamietanna, just checking in on this PR - if there's anything you need me to do, please let me know.

@jamesphillpotts-fr
Copy link
Contributor Author

Hi @jamietanna is there any chance if getting this PR moving again please?

@jamietanna
Copy link
Member

Would you mind making sure the latest main branch is merged in? Can't see GitHub Actions having run.

I've got a day off tomorrow which I'm planning on spending a good chunk on oapi-codegen, so hopefully will get to this after #1254

@jamesphillpotts-fr
Copy link
Contributor Author

No problem, I'll get onto that after some lunch :-)

@jamesphillpotts-fr
Copy link
Contributor Author

@jamietanna all up to date, tests pass locally - please trigger the github actions when you get a chance :-)

@jamesphillpotts-fr
Copy link
Contributor Author

Hi @jamietanna how did you get on with looking at this on Friday?

@jamietanna
Copy link
Member

@jamesphillpotts-fr would you say that this PR should also close off #1354 ?

@jamesphillpotts-fr
Copy link
Contributor Author

@jamietanna once x-go-type-name is added, I believe so, yes.

@jamietanna
Copy link
Member

Thanks for your patience!

@jamietanna jamietanna merged commit 311b23b into oapi-codegen:master May 18, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants