Repository navigation
Merged
Conversation
JVM tool for the Java loadout (#899): upstream release artifact run on our jdk through a /usr/bin wrapper, with Standalone tests that run the tool on a small real input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely identifies the package and the added version, which matches the main change in the pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
There was a problem hiding this comment.
Clean. The sha256 matches, the manifest carries the Add-Exports entries so plain java -jar works, and the tests check exact formatted output and --set-exit-if-changed.
- Nit,
build.sh:8:cpleaves the jar's mode to however it was fetched.install -m 0644, as in #922 and #923, makes it deterministic and consistent.
Holding approval only on the shared policy question: whether JVM tools may ship the upstream jar (with binary_from) instead of building from source, as AGENTS.md otherwise requires. This is being decided once for the whole batch.
Generated by Claude Code
Member
Author
|
Thanks @norrietaylor.
🤖 Generated with Claude Code |
norrietaylor
approved these changes
Oct 22, 2026
Member
There was a problem hiding this comment.
Approving under #970. The install -m 0644 nit is fixed and the branch is past #897. Please merge after #970.
Generated by Claude Code
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
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.
What
Adds
google-java-format1.37.0, the de-facto Java formatter. This is the first Tier 1 item in the #899 Java loadout checklist.google-java-format-1.37.0-all-deps.jarfrom GitHub releases. It is installed atusr/share/google-java-format/google-java-format.jarand run on ourjdkthrough a/usr/bin/google-java-formatwrapper (exec java -jar …).linux-x86-64andlinux-arm64. The jar is one architecture-independent artifact, so there is one sha256 and nomatch targetdispatch, and it runs on the same JDK as the rest of the Java tooling. The jar's manifest already carries theAdd-Exports: jdk.compiler/com.sun.tools.javac.*entries, so plainjava -jarworks without the--add-exportsflags the README lists for other launch modes. It needs a full JDK (thejdk.compilermodule) at version 21 or later (class file version 65); ourjdkis 25 (Temurin, since jdk: current LTS (meta over jdk-25); add jdk-25 (Temurin 25) and jdk-21 (Temurin 21) #897), which satisfies it.maven,gradleandjdtlsmake. Recorded asbinary_from.Apache-2.0.source_provenance = GithubRepo google/google-java-format. Tags arev<version>.834b2a0c…62f7a, computed locally and matching the GitHub release asset digest.Tests (Standalone)
formatformatsclass Hello{void f(){int x=1;if(x>0){x++;}}}and runsdiff -uagainst the exact Google-style output.check_moderuns--dry-run --set-exit-if-changed. It must exit non-zero on an unformatted file and zero on a formatted one.Verified
Built in a
pkgbuildsession (arm64) withmin package build --rebuild.min check --packages google-java-formatpassed every checker, includingstandalone tests,output types validandmissing runtime_deps. Negative control: changing the expected output tox--failedformatwith the real diff (-x--/+x++).amd64 is not separately run locally; it is left to CI.
Refs #899
🤖 Generated with Claude Code
Summary by CodeRabbit