-
Notifications
You must be signed in to change notification settings - Fork 1
Format Only Changes #197
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
base: master
Are you sure you want to change the base?
Format Only Changes #197
Conversation
SentryMan
commented
Feb 17, 2025
•
edited
Loading
edited
- sorts Context/Jex members
- adds auto-formatter
- format code with the google java style
I'm unconvinced by this. |
Is there any particular reason? Pros:
Cons:
|
For me, it fails aesthetically in too many common cases with too much indentation (double and quadrupedal indentation) and line breaking. There are formatters that don't aesthetically fail as much as google java style / basically closer to IntelliJ style. |
do you have one in mind that I can setup automatic formatting with? |
For me I couldn't care less about any particular code style, it's just that a standard format that can be enforced automatically makes contribution/reviewing easier. |
what do you think about https://github.com/palantir/palantir-java-format |
I have been trying it out actually, but yes it does kinda suck in some DSL cases [e.g. ebean query bean queries] so I'm not 100% sold on it. Still looking. |
okay, but are those DSL cases in this repo/pr? I'm not talking about changing what you use for application development. |
629092b
to
1eed5f9
Compare
@rbygrave do any of the diffs in this pr have any formatting issues that will prevent you from accepting the auto-formatter on this repo? |
I'm still trialing the IntelliJ plugin and that wasn't actually working as I expected yet [which I realise sounds weird, I thought it would "just work" but not really] ... plus I am wondering about the specifics on tabs/spaces ... plus pondering if formatting should actually fail a PR or autoformat. ... so those are the things that are still WIP in my mind yes, and I'd want to get this "right" rather than do this multiple times because we rushed and got it wrong etc. |
For me I will continue to use the google format in my ide because with this change I'm finally free to not care about formatting. This pr doesn't auto-format, it fails the PR with a message to use the mvn spotless apply goal. |
To speak plainly, the whole point of this pr is so that you don't have to setup/change any IDE/plugin settings. |
Well, it seems palantir isn't gonna fly, (text blocks don't work)
what are these other formatters? (and can they be used for auto formatting) |
prettier is not a good option since it requires a node installation and |
@rbygrave FWIW I use one of the following in my projects
All of the above are (or can) use Eclipse JDT (headless and only the formatting portion of ECJ). The first one you just give it an Eclipse formatting properties file. The second one has Springs eclipse formatting properties baked in. The third (Spotless) is like the first one but allows different file types and different formatters. All of the JStachio projects use spring-javaformat and all of my companies projects use the formatter-maven-plugin. I mostly do not care about formatting so long as the build goes ahead and fixes it for me however I don't like the google java format. I think indenting by 2 spaces because of line room ... is ... well just break up your code and don't indent that much or maybe not have names that are 40 characters long :). Intellij has a plugin to read Eclipse format: https://plugins.jetbrains.com/plugin/6546-adapter-for-eclipse-code-formatter Or you can use the Spotless Intellij plugin (which would then still call Eclipse formatter). While IntelliJ kicks Eclipse ass on most things it does not have nearly the flexibility last I checked as Eclipse formatter does. My recommendation is you crack open Eclipse like once and configure the format you want and export it. Then use the above plugins to read that format. What I do in terms of build is have two Maven profiles. The build machine always uses the profile that will check if the format is correct (usually by diff I think or checksum on multiple runs). Then I have another profile that will automatically reformat. In opensource projects I don't have this profile run unless enabled because people don't like build changing their code but in closed source I have the formatter run on every non-build-machine build. What happens is if someone runs the build it will fail instead and they are then told to rerun the build with some goal that will auto-reformat. I will say no auto formatting is better than Checkstyle (IMO is garbage and not needed today) but having auto formatting is worth it for consistency. BTW if the build fails because formatting was not correct ... that means they didn't rebuild before pushing. In a normal PR this will be fine since you wait till green before merge/rebase. |
08a8360
to
c879113
Compare
43ab282
to
3b7953b
Compare