Moving the baseline to Java 17 / Jakarta EE 10#739
Conversation
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
… versions available. Signed-off-by: Jacek Bilski <jacek@bilski.tech>
…ta EE 8 version. Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
…grading Quarkus to the newest 3.x version. Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
Signed-off-by: Jacek Bilski <jacek@bilski.tech>
|
Hi @Cali0707, any chances you could take a look at this soon? I have already the next PR ready, see jacekbilski#2. |
Cali0707
left a comment
There was a problem hiding this comment.
Thanks for working on this @jacekbilski - this is looking really good overall
I only found one issue to look into, then let's merge this and continue along with work for the 5.0.0 release
Are there other PRs you want to land for 5.0 ?
| response.setStatus(HttpStatus.OK_200); | ||
| response.write(true, ByteBuffer.wrap(body), callback); | ||
| } else { | ||
| response.setStatus(HttpStatus.NO_CONTENT_204); |
There was a problem hiding this comment.
@jacekbilski I think we need to call callback.succeeded() here? since we don't do response.write
There was a problem hiding this comment.
Hi. Good catch. I also now asked an AI agent to review the file, and it made the same remark -> fixed.
But what it also mentioned is, that this example using Jetty 12.1 is no longer "HTTP basic" example, as it's not using standard SDK things, but Jetty's own API. Should I rename the whole module? Makes sense actually.
There was a problem hiding this comment.
IMO the "basic http" part of this example comes from the BasicHttpServer.java
There was a problem hiding this comment.
Not really. In version 4.1.1 we were using for example javax.servlet.http.HttpServletRequest and javax.servlet.http.HttpServletResponse. You could call it "basic". Now it's pure Jetty.
OK, I see. Now we have definitely mixed two ways together. I'll extract the Jetty module, but feel free to drop the commit before merging if it's going too far.
It is "http basic". Even if it's using some Jetty-specific API, it's also using cloudevents-http-basic. Separate module doesn't make sense.
…is provided. This was also found by a coding agent. Signed-off-by: Jacek Bilski <jacek@bilski.tech>
As mentioned above, I have the next PR ready: jacekbilski#2. There, I'm also bumping Jakarta EE version up, along with some other things like Spring Boot. I guess that would be already too much for version 5. I leave the decision to you. |
Cali0707
left a comment
There was a problem hiding this comment.
LGTM, thanks @jacekbilski
This is the third PR of the series towards Java 17 / Spring Boot 4 / Quarkus 3 / Jakarta EE 11.
This PR is probably the biggest one of the series. We're now using Java 17 as a baseline. Also, old Jakarta EE 8 (packages starting with
javax.) is gone, the baseline is now Jakarta EE 10. All major projects are also upgraded to their versions matching or roughly matching Jakarta EE 10. That also means that we're still using Spring Boot 3, for example.I honestly didn't test it with Spring Boot 4 yet, but will try soon.