optimize: Apply Spotless with Palantir Java Format #7223

YongGoose · 2025-03-15T15:10:05Z

🔹 Related Document

https://lists.apache.org/thread/b4mmng4t0tog6bd58y7y9vnr02jn0q9f

🔹 Key Changes:

Configured Spotless Maven Plugin to automatically format Java files using Palantir Java Format.
- https://github.com/palantir/palantir-java-format
JDK version-based profile activation:
- JDK 8 to <11 → Uses palantirJavaFormat.version = 1.1.0
- JDK 11 to <21 → Uses palantirJavaFormat.version = 2.28.0
- JDK 21+ → Uses palantirJavaFormat.version = 2.39.0
Excluded specific directories/files from formatting:
- script/, generated/, antlr/mysql/parser/, antlr/mysql/antlr/, etc.
Integrated with Maven’s process-sources phase, ensuring formatting is applied during builds.
Enabled unused import removal, trailing whitespace trimming, and newline enforcement at EOF.

🔹 How to Verify:

1️⃣ Run Maven build to ensure Spotless is applied automatically:

mvn clean install

2️⃣ Manually apply formatting with:

mvn spotless:apply

3️⃣ Verify excluded directories remain unchanged.

🔹 Screen shots

ps. I found that applying Spotless resulted in 1,914 files being modified!!

YongGoose · 2025-03-15T23:31:17Z

@funky-eyes @xingfudeshi

What do you think? 🤔

slievrly · 2025-03-16T12:37:24Z

@YongGoose Can Spotless only work on the change files instead of all files?

codecov · 2025-03-16T12:41:16Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.24%. Comparing base (6083fa1) to head (0b3d1e4).
Report is 6 commits behind head on 2.x.

Additional details and impacted files

@@            Coverage Diff            @@
##                2.x    #7223   +/-   ##
=========================================
  Coverage     54.23%   54.24%           
- Complexity     7267     7269    +2     
=========================================
  Files          1178     1178           
  Lines         41952    41952           
  Branches       4923     4923           
=========================================
+ Hits          22753    22756    +3     
+ Misses        17048    17045    -3     
  Partials       2151     2151

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

YongGoose · 2025-03-16T12:48:36Z

Can Spotless only work on the change files instead of all files?

No, Spotless can be applied to all files as well.

funky-eyes · 2025-03-19T06:31:41Z

I think this approach is very effective. Although it involves modifying many files, I believe as long as the CI passes, there won't be any significant issues. Even though it may conflict with other PRs, I think this cost is worthwhile.

YongGoose · 2025-03-19T06:35:21Z

I think this approach is very effective. Although it involves modifying many files, I believe as long as the CI passes, there won't be any significant issues. Even though it may conflict with other PRs, I think this cost is worthwhile.

Thank you for your comment!

However, after discussing with @slievrly on the mailing lists, we decided to apply spotless only to the modified files, so it looks like we won’t be modifying many files!

Although there is the drawback that spotless won’t be applied to unmodified files, after weighing the trade-offs with the git history, we determined that it’s better to only modify the changed files.

funky-eyes · 2025-03-20T03:29:04Z

I think this approach is very effective. Although it involves modifying many files, I believe as long as the CI passes, there won't be any significant issues. Even though it may conflict with other PRs, I think this cost is worthwhile.

Thank you for your comment!

However, after discussing with @slievrly on the mailing lists, we decided to apply spotless only to the modified files, so it looks like we won’t be modifying many files!

Although there is the drawback that spotless won’t be applied to unmodified files, after weighing the trade-offs with the git history, we determined that it’s better to only modify the changed files.

It would be great if this can be achieved, but when I run the mvn spotless:apply command locally, how can I ensure that only the files I have modified are formatted?

YongGoose · 2025-03-20T06:40:28Z

It would be great if this can be achieved, but when I run the mvn spotless:apply command locally, how can I ensure that only the files I have modified are formatted?

I believe it can be done by using the git diff feature with ratchet.
I haven’t actually implemented it yet, but it does seem feasible.

https://github.com/diffplug/spotless/tree/main/plugin-maven#how-do-i-preview-what-mvn-spotlessapply-will-do

funky-eyes · 2025-03-20T07:22:30Z

It would be great if this can be achieved, but when I run the mvn spotless:apply command locally, how can I ensure that only the files I have modified are formatted?

I believe it can be done by using the git diff feature with pre-commit hook. I haven’t actually implemented it yet, but it does seem feasible.

https://github.com/diffplug/spotless/tree/main/plugin-maven#how-do-i-preview-what-mvn-spotlessapply-will-do

Ok, I hope this solution is feasible. If it's not, I don't mind modifying all the files; I don't think it will have too much of an impact.

YongGoose · 2025-03-20T09:56:41Z

pom.xml

+                                    <exclude>**/antlr/mysql/antlr/*.*</exclude>
+                                    <exclude>**/antlr/mysql/stream/ANTLRNoCaseStringStream.java</exclude>
+                                </excludes>
+                                <ratchetFrom>HEAD</ratchetFrom>


@slievrly @funky-eyes @xingfudeshi

I configured <ratchetFrom>HEAD</ratchetFrom> so that only the modified files are formatted by comparing them with the currently checked-out commit (HEAD).

@slievrly @funky-eyes @xingfudeshi

I configured <ratchetFrom>HEAD</ratchetFrom> so that only the modified files are formatted by comparing them with the currently checked-out commit (HEAD).

Good job! I will pull this PR to my local machine for related testing

@funky-eyes

Currently, our baseline is set to HEAD, which means that only files changed since the last commit are considered. This creates a potential issue: if code with incorrect formatting gets committed, checkstyle will catch it, but Spotless won’t apply its fixes, and we lose the intended benefit.

To address this, we tried switching the baseline from HEAD to origin/2.x. However, the branch wasn’t recognized. 🥹

What do you think?

@funky-eyes

Currently, our baseline is set to HEAD, which means that only files changed since the last commit are considered. This creates a potential issue: if code with incorrect formatting gets committed, checkstyle will catch it, but Spotless won’t apply its fixes, and we lose the intended benefit.

To address this, we tried switching the baseline from HEAD to origin/2.x. However, the branch wasn’t recognized. 🥹

What do you think?

Hi, I pulled your code and followed the steps. Seems to work!

1. git checkout feature/apply-spotless 2. Change the value of ratchetFrom from HEAD to feature/apply-spotless 3. Add some spaces in the method of Java class, like this

4. mvn spotless:check 5. Then I got the outputs like this

@funky-eyes

Currently, our baseline is set to HEAD, which means that only files changed since the last commit are considered. This creates a potential issue: if code with incorrect formatting gets committed, checkstyle will catch it, but Spotless won’t apply its fixes, and we lose the intended benefit.

To address this, we tried switching the baseline from HEAD to origin/2.x. However, the branch wasn’t recognized. 🥹

What do you think?

I think it can be done this way

In other words, unless we format and commit all the code at once, we will not be able to prompt the user to run mvn spotless:apply. What if we change the HEAD to 2.x?

What if we change the HEAD to 2.x?

I think this approach is a good one.

However, when I tried running it locally, the behavior was different from what I expected, so I'm reconsidering it.
To explain in more detail, switching to origin/2.x should apply changes only to the files that have been modified from 2.x.

Even though there were no changes in my local branch, some recently committed files in 2.x were also modified.

Still, I think setting it to 2.x is a better approach than setting it to HEAD.

In this commit 4f07580 i changed ratchet from to origin/2.x

Then below error occured.

Error: Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.44.3:apply (default) on project seata-parent: Execution default of goal com.diffplug.spotless:spotless-maven-plugin:2.44.3:apply failed: No such reference 'origin/2.x' -> [Help 1]

I noticed that after setting it to 2.x, the number of changed files is quite large, unless we are willing to accept changes to all files. Otherwise, I think we can initially set it to HEAD, merge this PR, and then create an issue to discuss the remaining problems.
@slievrly What do you think?

xingfudeshi · 2025-03-27T17:38:59Z

It would be great if this can be achieved, but when I run the mvn spotless:apply command locally, how can I ensure that only the files I have modified are formatted?

I believe it can be done by using the git diff feature with pre-commit hook. I haven’t actually implemented it yet, but it does seem feasible.

https://github.com/diffplug/spotless/tree/main/plugin-maven#how-do-i-preview-what-mvn-spotlessapply-will-do

Ok, I hope this solution is feasible. If it's not, I don't mind modifying all the files; I don't think it will have too much of an impact.

I agree with your point .

slievrly · 2025-04-08T16:24:47Z

pom.xml

+                        </configuration>
+                        <executions>
+                            <execution>
+                                <goals>


How it works in github action workflow, I didn't see this profile id included in workflow.In addition to checking if the format is as expected, is it possible to automatically format changes in the workflow and commit them to PR?

This doesn't work in GitHub Actions.

Automatically formatting and committing changes seems more suitable for a git hook, which would require users to modify their local .git folder.

For now, in this PR, I’ve applied Spotless.
I’m planning to explore additional improvements in a separate PR.

slievrly

LGTM

YongGoose added 2 commits March 15, 2025 23:48

init spotless

32a5e7e

Merge branch '2.x' into feature/apply-spotless

de6cf2e

Merge branch '2.x' into feature/apply-spotless

73b865c

xingfudeshi self-requested a review March 17, 2025 03:36

Merge branch '2.x' into feature/apply-spotless

c294714

YongGoose added 2 commits March 20, 2025 18:39

Add ratchetFrom

32b55f7

Add changes

661d18a

YongGoose marked this pull request as ready for review March 20, 2025 09:49

change ratchetFrom to HEAD

f7a0266

YongGoose commented Mar 20, 2025

View reviewed changes

xingfudeshi and others added 2 commits March 21, 2025 09:51

Merge branch '2.x' into feature/apply-spotless

fc7770f

Merge branch '2.x' into feature/apply-spotless

3d5b4eb

funky-eyes added this to the 2.4.0 milestone Mar 27, 2025

funky-eyes added the type: feature Category issues or prs related to feature request. label Mar 27, 2025

Merge branch '2.x' into feature/apply-spotless

5c30142

YongGoose force-pushed the feature/apply-spotless branch from ad8bdcd to 749319e Compare March 31, 2025 06:45

change ratchetFrom to origin/2.x

3bee817

YongGoose force-pushed the feature/apply-spotless branch from 749319e to 3bee817 Compare March 31, 2025 06:53

revert changes

a31ddf8

YongGoose force-pushed the feature/apply-spotless branch from ba28448 to a31ddf8 Compare March 31, 2025 06:57

YongGoose added 4 commits March 31, 2025 15:58

change ratchetFrom to origin/2.x

4f07580

change ratchetFrom to 2.x

0e71c8d

change ratchetFrom to refs/2.x

ffa856b

revert changes

0b3d1e4

slievrly reviewed Apr 8, 2025

View reviewed changes

YongGoose requested a review from slievrly April 10, 2025 07:56

slievrly changed the title ~~feature: Apply Spotless with Palantir Java Format~~ optimize: Apply Spotless with Palantir Java Format Apr 12, 2025

slievrly approved these changes Apr 12, 2025

View reviewed changes

slievrly merged commit 70b0ade into apache:2.x Apr 12, 2025
9 checks passed

YongGoose mentioned this pull request Apr 23, 2025

Improve Spotless Integration: Enhance CI Workflow #7308

Open

optimize: Apply Spotless with Palantir Java Format #7223

optimize: Apply Spotless with Palantir Java Format #7223

Uh oh!

Conversation

YongGoose commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔹 Related Document

🔹 Key Changes:

🔹 How to Verify:

🔹 Screen shots

Uh oh!

YongGoose commented Mar 15, 2025

Uh oh!

slievrly commented Mar 16, 2025

Uh oh!

codecov bot commented Mar 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

YongGoose commented Mar 16, 2025

Uh oh!

funky-eyes commented Mar 19, 2025

Uh oh!

YongGoose commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

funky-eyes commented Mar 20, 2025

Uh oh!

YongGoose commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

funky-eyes commented Mar 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xingfudeshi commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slievrly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

YongGoose commented Mar 15, 2025 •

edited

Loading

codecov bot commented Mar 16, 2025 •

edited

Loading

YongGoose commented Mar 19, 2025 •

edited

Loading

YongGoose commented Mar 20, 2025 •

edited

Loading

xingfudeshi commented Mar 27, 2025 •

edited

Loading