test: try Palantir Java Format (120-col) vs GJF AOSP - #135
Draft
arena-ai-coding-agent[bot] wants to merge 1 commit into
Draft
test: try Palantir Java Format (120-col) vs GJF AOSP#135arena-ai-coding-agent[bot] wants to merge 1 commit into
arena-ai-coding-agent[bot] wants to merge 1 commit into
Conversation
…ormat
Swaps googleJavaFormat("1.15.0").aosp() for a pinned
palantirJavaFormat("2.71.0") so we can compare 120-column Palantir
layout against the current 100-column GJF AOSP style.
Palantir is a google-java-format fork with the same 4-space indent but
120-column lines and wrapping tuned for lambdas, streams and fluent
chains -- which is what most of our Commands.sequence(...) trees and
Trigger bindings look like.
The version is pinned rather than left to Spotless's default: Spotless
6.25.0 selects a Palantir version per JVM and would hand JDK 17 an
ancient 2.28.0. 2.71.0 runs on JDK 17 (Java 11 bytecode; the Java 21
AST visitor only loads reflectively on a 21+ JVM), so WPILib 2026,
student machines and the wpilib/roborio-cross-ubuntu CI image are fine.
toggleOffOn(), removeUnusedImports(), trimTrailingWhitespace(),
endWithNewline() and the .gradle/.xml/.md formatters are unchanged.
Docs updated to describe 120-col Palantir instead of GJF/AOSP/100-col.
NOTE: this commit contains the config + docs change only. The mechanical
Java rewrite still needs `./gradlew spotlessApply` run and pushed onto
this branch; spotlessCheck will fail until then.
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Contributor
|
Review: The Palantir Java Format change looks solid! 120 columns solve the type name issues we mentioned in the docs. Should we proceed with adopting it during the next code review window? |
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 this is
An experiment, not a style decision. This swaps Spotless's Java formatter from
googleJavaFormat("1.15.0").aosp()(4-space, 100 cols) to a pinnedpalantirJavaFormat("2.71.0")(4-space, 120 cols) so we can look at real diffs of our owncode and decide whether we like it.
Branched from
mainon purpose, not from #133 (the GJF 1.15.0 → 1.28.0 bump), so thediff here is Palantir vs. the style we ship today rather than Palantir vs. a bumped GJF.
Important
Do not squash-merge unless we decide to adopt it — the mechanical rewrite touches
nearly every Java file and squashing nukes
git blameacross the whole codebase. If weadopt it, merge with a regular merge commit and add the reformat commit to
.git-blame-ignore-revs.Warning
This PR is currently config + docs only. The mechanical Java rewrite has not been
committed yet — see Status at the bottom.
spotlessCheckwill fail untilsomeone runs
./gradlew spotlessApplyand pushes onto this branch.Why Palantir
Palantir Java Format is a fork of
google-java-format. Same 4-space indent we already use, but:
WantedSuperState.LAUNCH_WITH_SQUEEZE_WITH_NO_DELAY,SwerveDrivetrainConstants,robotSuperStructure.setStateCommand(...). At 100 cols GJF breaks these mid-expression;at 120 most of them fit on one line.
Commands.sequence(...).withName(...)chains are most ofAuton.javaandCoordinator.java. GJF's AOSP mode indents continuations 8 spaces froman already-deep position, which pushes arguments to column 24+ before any content.
Commands.runOnce(() -> { ... })andTriggerbindings areeverywhere in this repo; Palantir's wrapping rules were written with exactly that shape
in mind.
build.gradle. No style file to maintain.Version pin and the JDK 17 constraint
Pinned to
palantirJavaFormat("2.71.0").This has to run on JDK 17 — WPILib 2026, student machines, and the CI image
wpilib/roborio-cross-ubuntu. 2.71.0 satisfies that:Runtime.version().feature() >= 21. On a 17 JVM it stays on the Java 14 visitor, whichhandles records, switch expressions, and
instanceofpatterns — everything this repoactually uses (
ShotCalculator.ShootingParameters,SuperStructure'sswitch (wantedSuperState), etc.).since dropped per-JVM version gating entirely, noting Palantir is "compatible with Java
11+" (diffplug/spotless#2686).
The pin is deliberate — don't remove it. Spotless 6.25.0 picks a Palantir version per
JVM and would silently hand JDK 17 a very old 2.28.0. Also don't bump to a version
that requires JDK 21 to run; there is a comment in
build.gradlesaying so.Out of scope, deliberately: no Spotless 7/8 bump, no GJF
formatJavadoc, no changes tothe
.gradle/.xml/.md/.gitignoreformatters.toggleOffOn(),removeUnusedImports(),trimTrailingWhitespace()andendWithNewline()are all kept,so
// spotless:off/// spotless:onstill works.Before / after
Real snippets from this repo. "Before" is current
main; "after" is what Palantir's120-col rules produce.
Auton.java— the launch sequenceGJF AOSP indents
Commands.sequencearguments to column 24 and still breaks thesetStateCommandline, because at 8-space continuation indent it lands at 98 chars —just under the 100-col limit, with nothing left over.
Note this one barely moves — which is itself useful information. The win shows up where
lines are currently forced to break.
SuperStructure.java— switch expression with long enum constantsThis is the clearest case. At 100 cols GJF has to split the enum constant across two
lines, orphaning
.LAUNCH_WITH_SQUEEZE_WITH_NO_DELAYonto its own line. Rejoined the lineis 106 chars — fits comfortably at 120.
The ternary right above it is 142 chars rejoined, so it still wraps at 120 — but it wraps
on the
?/:boundaries, which is where you'd want it anyway:SuperStructure.java— lambda in a fluent chainThe deep continuation indent before
() -> {is the pattern worth judging across thewhole rewrite — it's what Palantir's lambda rules are meant to improve.
How to revert
This is designed to be thrown away.
mainchanges.git revert -m 1 <merge-commit-sha>, orgit revert <sha>for the reformat commit and the config commit. Because the rewrite ispurely mechanical, the revert is clean — no hand-resolution needed.
build.gradlehunk back togoogleJavaFormat("1.15.0").aosp()and re-run./gradlew spotlessApply; thatregenerates the old layout without touching anything else.
Status
build.gradle—googleJavaFormat("1.15.0").aosp()→palantirJavaFormat("2.71.0"), comments updatedAGENTS.md,docs/tools/build-tools.md,docs/tools/gradle.md,docs/coding-conventions/code-style.mdtoggleOffOn()/removeUnusedImports()/trimTrailingWhitespace()/endWithNewline()and the non-Java formatters left alone./gradlew spotlessApplymechanical rewrite — not yet committed./gradlew spotlessCheckgreen on JDK 17./gradlew buildgreen on JDK 17The last three need someone on a machine with Maven Central access:
Keep that reformat as its own commit with no feature edits mixed in, so it stays
trivially revertable and easy to add to
.git-blame-ignore-revslater.