Upgrade to Java 27 & small improvements - #570
MartelliEnrico wants to merge 8 commits into
Conversation
- Compact Object Headers are on by default, so the jre creation was simplified - C1 is the new GC, and for the bot usage should be better than Generational Shenandoah
These checks enable the new jdk nullability data, so nullable lib methods are properly annotated now
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe project now uses Java 27 in its build, CI, Docker builder, and setup instructions. The Gradle build updates jlink and nullability settings and packages an OpenTelemetry agent. Application code, the Windows launcher, and test helpers also change. ChangesProject updates
Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to The OpenTelemetry agent is now loaded by a relative path. Starting the app from a directory other than its install location, or through Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The bundled agent is selected using a working-directory-relative filename rather than its installed location. Launching from another directory can fail or, if another actor controls that directory, load unintended code with the bot’s privileges. Agent execution also expands beyond Docker distributions. The default container layout limits the first risk, but packaged startup and failure-recovery behavior remain unverified. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 10 files. (2 skipped: 2 unsupported.)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
|
| "-c:v", "libvpx-" + VP9_CODEC, | ||
| "-row-mt", "1", | ||
| "-threads", "2", | ||
| "-filter_threads", "1", |
There was a problem hiding this comment.
This is a parallel of the -row-mt 1 settings, which limit the processing to 1 cpu for the codec, while -filter_threads 1 limits the processing of the filters to 1 cpu. All this to avoid too many processes running inside the Docker image
| # bump: Qodana /qodana-jvm-community:([\d.]+)/ docker:jetbrains/qodana-jvm-community|/^[\d]{4}\.\d+$/|sort | ||
| linter: jetbrains/qodana-jvm-community:2026.2 | ||
| projectJDK: temurin-26 | ||
| projectJDK: temurin-27 |
There was a problem hiding this comment.
It seems like it isn't available yet on their side
There was a problem hiding this comment.
We just need to wait for Qodana to catch up
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
0fd6f75e-b040-44b5-b29f-76dd03185ee2
📒 Files selected for processing (6)
Dockerfilebuild.gradle.ktsbuildSrc/src/main/java/com/github/stickerifier/stickerify/DownloadOpenTelemetryAgentTask.javagradle/libs.versions.tomlsrc/main/java/com/github/stickerifier/stickerify/bot/Stickerify.javasrc/main/java/com/github/stickerifier/stickerify/process/OsConstants.java
💤 Files with no reviewable changes (1)
- Dockerfile
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
| application { | ||
| mainClass = "com.github.stickerifier.stickerify.runner.Main" | ||
| applicationDefaultJvmArgs = listOf("-XX:+UseCompactObjectHeaders", "-XX:+UseShenandoahGC", "-XX:ShenandoahGCMode=generational", "--enable-final-field-mutation=ALL-UNNAMED") | ||
| applicationDefaultJvmArgs = listOf("--enable-final-field-mutation=ALL-UNNAMED", "-javaagent:" + openTelemetryAgent.get().destinationFile.get().asFile.name) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Resolve the agent path for each launch mode.
applicationDefaultJvmArgs applies to gradle run and the distribution launchers. The agent JAR is added only to the distribution, while -javaagent:opentelemetry-javaagent.jar resolves relative to the process working directory. If gradle run starts outside that directory, or a user starts an installed launcher from elsewhere, the JVM cannot find the agent and the application does not start. Make run depend on the download task and give it the build output path. Resolve the packaged agent relative to the installation directory in the launch scripts. (docs.gradle.org)
| var targetFile = getDestinationFile().get().getAsFile(); | ||
|
|
||
| var _ = targetFile.getParentFile().mkdirs(); | ||
| try (var inputStream = URI.create(urlString).toURL().openStream()) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Set connect and read timeouts for the agent download.
If the release connection stalls, openStream() can wait indefinitely because Java 27 leaves both timeouts disabled by default. This can leave the distribution build waiting until an operator cancels it. Open a connection and set both timeouts before reading the JAR. (docs.oracle.com)
| } | ||
|
|
||
| val openTelemetryAgent = tasks.register<DownloadOpenTelemetryAgentTask>(DownloadOpenTelemetryAgentTask.DEFAULT_TASK_NAME) { | ||
| description = "Downloads the OpenTelemetry agent for the distribution package." |
There was a problem hiding this comment.
Do we need to define the description both here and inside the task implementation?
There was a problem hiding this comment.
In the task implementation is best practice, here is a warning if not provided
Summary by CodeRabbit
Compatibility
Media
Bug Fixes