OpenApi 3.1.2 (Fixes #728)#730
Open
re-thc wants to merge 14 commits into
Open
Conversation
Contributor
Author
|
Doesn't look like this change broke CI. Edit: maybe due to caching (since this drops oai 3.0. old things would break). Edit 2: dirty file locally :( |
Contributor
|
Reposting test stacktrace to save the trouble: |
Contributor
Author
Fixed |
Contributor
|
So for myself I'm good with this PR. So I think the steps to are something like:
|
Collaborator
|
The only thing is how the other nullability PR is affected by this |
Contributor
Author
It's either this 1st and update or that 1st and update. Take your pick :). Maybe base it on which direction is easier. |
Contributor
|
I think the plan should be:
|
Collaborator
You could also go the other way and Merge the null handling PR |
Contributor
Yup, I'm good with that. |
cbarlin
added a commit
to cbarlin/avaje-http
that referenced
this pull request
Apr 1, 2026
Based on the discussion in avaje#743, there was mixed feelings on the shading being done for the generators. I figured since there is a major version on the horizon (based on discussions in avaje#730), what we could do is: * Isolate out the prism generation, to help ensure we don't end up with unintentional dependencies in the actual generators (since the generator would fail compilation in this repo if you tried) * Make the "main" versions of the processors no longer shaded * Publish separate "-shaded" versions of the generators that are shaded The idea being if you are using `proc=full` and/or having issues with the dependencies for the generator not coming through, you can use the `-shaded` versions (e.g. instead of `avaje-http-client-generator` you'd use `avaje-http-client-generator-shaded`). If (or when) shading becomes problematic, then we can easily remove those shaded versions from being published. There's not really a "solid reason" for doing this now rather than waiting to see what happens with `proc=full`. It's just something that was discussed, and could be added to the list of potentially breaking changes in that next major. During this though, I did notice a couple of things: * The test pipeline doesn't test on Java 11, despite that being the minimum (?) Java version * The main `http-api` has a dependency on the `http-inject-plugin` which has a dependency back on the `http-api` and also only builds on Java 21+ (while `http-api` will try to build on Java 11) * The `http-api` had a (`static`/`optional`) dependency on javalin, but there's a `http-api-javalin` that could house it and forward on the `http-api` dependency * Both Avaje SPI and Avaje Prisms publish their annotations depending on their processor, rather than the other way around, which seems incorrect. I'm not sure if Maven 4 will have a problem with the annotations being `jar` and the dependency being `processor`. You can't have the annotations as both since duplicate G/A/V is not permitted, but even if it doesn't it "feels like" it should be the reverse (processor depends on annotations) I've marked this as draft/rfc because I want to know: * Is the separate shaded versions an OK compromise? * Is there a preferred path for dealing with the circular dependency? I also haven't finished: * all the shaded versions, * patching the tests so they use a mix of non-shaded and shaded generators (so we can be sure we don't break the shaded versions) * testing the Java 11 addition to the build process But I figured I'd wait for the discussion before doing too much more.
…_FIELDS - Remove types from IGNORED_FIELDS - Schema<?> propSchema, needs to be final for use in lambda
Contributor
|
@re-thc I just resolved some merge conflicts and I would like you to have a look over those commits. It looked like a relatively simple conflict resolution, but then I make some mistakes and then later hit some failing tests and had to adjust test expectations for everything to pass. It looks ok but if you can review those commits some time that would be great - thanks !! |
Contributor
Author
|
@rbygrave took a look along with the null changes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OpenApi 3.1.2 (Fixes #728)
As per the comment this is a "just do it" version. I'll leave the major version handling for another more knowledgeable.