-
Notifications
You must be signed in to change notification settings - Fork 422
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
codegen: Improve enum support #3861
codegen: Improve enum support #3861
Conversation
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/SchemaGenerator.scala
Show resolved
Hide resolved
@@ -1 +1 @@ | |||
sbt.version=1.3.13 | |||
sbt.version=1.10.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumping sbt fixes sbt scripted
on Java 18+
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/EndpointGenerator.scala
Show resolved
Hide resolved
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/EndpointGenerator.scala
Outdated
Show resolved
Hide resolved
Sorry this pr conflates a few things together, but I didn't have the energy to split it out only to resolve conflicts down the line.. I think it represents a decent improvement, though, and hopefully none of the functionality is really contentious. |
3aebac2
to
cf88667
Compare
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/ClassDefinitionGenerator.scala
Outdated
Show resolved
Hide resolved
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/EndpointGenerator.scala
Outdated
Show resolved
Hide resolved
openapi-codegen/core/src/main/scala/sttp/tapir/codegen/EnumGenerator.scala
Outdated
Show resolved
Hide resolved
Impressive! A few nitpick comments so far, I'll finish the review later today or tomorrow. |
Thanks for the comments 🙏 all very sensible suggestions (especially around that |
Test failure seems to be... not sure exactly. It ends with
-- Anyway I think the tests have generally been less flakey of late and I appreciate that, but I mentioned the observation in case it helped shed some light on what the (presumably timing) issue actually is. |
@hughsimpson Sorry for the flaky tests, I'm still investigating what's causing these freezes #3827 |
I guess this part of docs can be updated? https://tapir.softwaremill.com/en/latest/generator/sbt-openapi-codegen.html#json-support
Particularly "Other forms of OpenApi enum are not currently supported."? |
…ests for various libs to scripted tests
94eb307
to
c384b9d
Compare
@hughsimpson Thanks again, looks good! |
Various improvements to enum support in codegen:
* Note that Option[List[T]] is only supported for unexploded params -- for exploded params, there's no distinction betwen an empty list and an absent value; so the
required
field will be ignored for exploded query params.Also adds some support for
explode
on query params; will now default to explode = true (as per the spec), but also supportsexplode = false
. It still only supports arrays inform
style for now.Since the generator runs with scala 2.12 and does not have access to
sttp.tapir.model.CommaSeparated
I've had to implementcase class CommaSeparatedValues
andcase class ExplodedValues
in the generated files, which is a shame, but should be okay...