search

apache / arrow

Apache Arrow is a multi-language toolbox for accelerated data interchange and in-memory processing
https://arrow.apache.org/
Apache License 2.0
14.3k stars 3.47k forks source link

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

Open lidavidm opened 1 year ago

lidavidm commented 1 year ago

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 commented 1 year ago

+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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

We can't rewrite history.

lidavidm commented 1 year ago

We can use that feature for ignoring commits for blame.

rtadepalli commented 1 year ago

Ah, completely forgot that filter branch rewrites history :man_facepalming: