search

casid / jte

Secure and speedy templates for Java and Kotlin.
https://jte.gg
Apache License 2.0
737 stars 54 forks source link

Use Java modern features/APIs #357

Closed marcospereira closed 2 months ago

marcospereira commented 2 months ago

What?

This runs OpenRewrite Java 17 migration recipe to adopt new/modern Java language features and APIs.

A few of the more extensive changes involve:

How?

I've added the plugin described in OpenRewrite docs but executed it only for projects that include jte-parent as a parent.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 91.30435% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 91.31%. Comparing base (9320ca7) to head (a367477).

Files Patch % Lines
...ain/java/gg/jte/models/runtime/StaticJteModel.java 0.00% 1 Missing and 1 partial :warning:
...rc/main/java/gg/jte/compiler/TemplateCompiler.java 83.33% 0 Missing and 1 partial :warning:
...extensionsupport/ExtensionTemplateDescription.java 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #357 +/- ## ============================================ - Coverage 91.38% 91.31% -0.08% + Complexity 1219 1217 -2 ============================================ Files 76 76 Lines 3168 3165 -3 Branches 493 490 -3 ============================================ - Hits 2895 2890 -5 - Misses 163 164 +1 - Partials 110 111 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

casid commented 2 months ago

@marcospereira this looks good to me, except that I would like to revert the record commit.

It doesn't bring much benefit compared to the dumb data classes that are already there (except adding getters that are not really needed).

Especially stuff like public record DirectoryCodeResolver is really bad. This is not a data class, but public API. The change also breaks the public API for every user who is using getRoot() on that class for no reason.

marcospereira commented 2 months ago

It doesn't bring much benefit compared to the dumb data classes that are already there (except adding getters that are not really needed).

Yeah, I was also in doubt about it, mainly because of the compatibility issue, and that is why the PR was a draft.

It is now gone. :-)

casid commented 2 months ago

Awesome! Thank you for the clean-up :-)