Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Java] Replace checkstyle with google-java-format or another formatter? #213

Closed
lidavidm opened this issue Feb 27, 2023 · 8 comments
Closed
Labels
Type: enhancement New feature or request
Milestone

Comments

@lidavidm
Copy link
Member

Describe the enhancement requested

checkstyle is unfortunately only a linter and cannot auto-format. google-java-format (or possibly some other plugin like clang-format) can be run to check and reformat files. google-java-format additionally integrates with IntelliJ. This makes development smoother.

However, we would have to reformat the entire codebase. Possibly, we can do this module-by-module.

Component(s)

Developer Tools, Java

@aiguofer
Copy link
Contributor

+1! Coming from the python world, where black is almost a standard now, I haven't had to think about formatting in a long time. I simply set my editor/IDE to auto-format on save and it always passes linting.

I think I spent at least 15 minutes the other day manually fixing little things to make checkstyle happy; I would love to only have to think about the code and not formatting.

Another interesting option is https://github.com/jhipster/prettier-java.

I see that pre-commit is already part of the project and dev process. One potential way forward could be to enable pre-commit to do the java auto-formating only on changed files. The downside of this approach would be that PRs would be harder to look through until all files are re-formatted.

@lidavidm
Copy link
Member Author

Unfortunately, CI relies on archery (which will recheck every file), not pre-commit, so that's not quite an option

prettier may be nice if we already use prettier elsewhere (I don't think we do), also not sure if it has integration with typical Java IDEs

In any case any of {google-java-format, clang-format, prettier} would probably be better than checkstyle

@lidavidm
Copy link
Member Author

From a quick glance, I would probably feel most comfortable with google-java-format (all Java, maintained). prettier-java looks to have its own Java parser and I'm not sure how much clang-format's Java support is used (or if it'll keep up with Java syntax changes, not that we can use them for a while yet...)

@rtadepalli
Copy link
Contributor

Thoughts on using filter branch to apply formatting without losing git blame? There are a fair amount of commits, and it'd take a while to get through all of them...

@lidavidm
Copy link
Member Author

We can't rewrite history.

@lidavidm
Copy link
Member Author

We can use that feature for ignoring commits for blame.

@rtadepalli
Copy link
Contributor

Ah, completely forgot that filter branch rewrites history 🤦‍♂️

@assignUser assignUser transferred this issue from apache/arrow Nov 26, 2024
@lidavidm
Copy link
Member Author

We already integrated Spotless.

@lidavidm lidavidm added this to the 18.2.0 milestone Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants