Skip to content

Move checkstyle cache to project root directory - #1350

Open
wendigo wants to merge 1 commit into
airlift:masterfrom
wendigo:user/serafin/add-checkstyle-cache
Open

wendigo wants to merge 1 commit into
airlift:masterfrom
wendigo:user/serafin/add-checkstyle-cache

Conversation

@wendigo

@wendigo wendigo commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Airbase contribution check list

  • Git commit messages follow https://cbea.ms/git-commit/.
  • All automated tests are passing.
  • Pull request was categorized using one of the existing labels.

@wendigo
wendigo requested review from electrum and martint August 26, 2026 14:25
@wendigo

wendigo commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

This makes subsequent Trino builds faster (even when cleaned):

After:

[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:15 min (Wall Clock)
[INFO] Finished at: 2026-08-26T16:24:39+02:00
[INFO] ------------------------------------------------------------------------

Before:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  02:00 min (Wall Clock)
[INFO] Finished at: 2026-08-26T16:28:15+02:00
[INFO] ------------------------------------------------------------------------

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e0fa9bf8-025b-4d77-b464-73773db415eb

📥 Commits

Reviewing files that changed from the base of the PR and between c187f9d and 5611aec.

📒 Files selected for processing (2)
  • .gitignore
  • airbase/pom.xml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Maven configuration adds a version-keyed Checkstyle cache path and assigns it to the maven-checkstyle-plugin through cacheFile. The .gitignore file ignores .checkstyle-cache/ directories.

Merge Risk: ⚪ Minimal · up to 5611a

The change moves the Checkstyle cache into a version-keyed project-root directory and ignores it from source control; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 89836248-39aa-4f90-aebe-e9992a2cab40

📥 Commits

Reviewing files that changed from the base of the PR and between 0294a56 and c187f9d.

📒 Files selected for processing (1)
  • airbase/pom.xml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread airbase/pom.xml Outdated
@wendigo
wendigo force-pushed the user/serafin/add-checkstyle-cache branch from c187f9d to 77716d4 Compare August 26, 2026 14:51
@wendigo

wendigo commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author
_Users_mateusz gajewski_Projects_src_github com_trinodb_trino_target_timeline_build-report html

@wendigo
wendigo requested review from dain and findepi August 26, 2026 20:40
@wendigo
wendigo force-pushed the user/serafin/add-checkstyle-cache branch from 77716d4 to 5611aec Compare August 31, 2026 07:25
@wendigo wendigo changed the title Move checkstyle cache to .m2 Move checkstyle cache to project root directory Aug 31, 2026
Comment thread airbase/pom.xml
<!-- checkout-specific so concurrent builds of separate checkouts never share -->
<!-- a cache file (Checkstyle rewrites it without locking), and keyed by the -->
<!-- Checkstyle version since the cache does not track the version on its own. -->
<air.checkstyle.cache-file>${maven.multiModuleProjectDirectory}/.checkstyle-cache/${project.groupId}/${project.artifactId}-${dep.checkstyle.version}.cache</air.checkstyle.cache-file>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like quicker builds. And I very much dislike how much time checkstyle takes on a clean (Trino) biuld.
But i also like the fact that "clean && build" is "a clean build", from well defined state. It could be very confusing if someone couldn't reproduce a CI failure despite running ./mvnw clean first.

I know the problem is pre-existing because of .node and node_modules directories, but I am not convinced the gain is worth the added complexity.

Maybe we should hear others opinion before merging?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would I need to "reproduce CI failure" around checkstyle?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants