Replace spotbugs with errorprone #1073
Conversation
jqno
force-pushed
the
replace-spotbugs-with-errorprone
branch
from
535f47f to
09e4ded
Compare
| @@ -241,10 +239,38 @@ | |||
| <artifactId>maven-compiler-plugin</artifactId> | |||
| <configuration> | |||
| <showWarnings>true</showWarnings> | |||
| <fork>true</fork> | |||
There was a problem hiding this comment.
Hi @jqno, out of curiosity, what made you go with fork=true instead of having a .mvn/jvm.config configuration?
I'm asking because I'm introducing ErrorProne too, and I picked the jvm.config direction so far, so I'm curious about your reasoning 🙂
There was a problem hiding this comment.
😄
First I want to say, I don't like that we have to add all these directives in the first place. But since we have to... I picked this approach because this way, I can keep it inside the codebase, and anybody who wants to clone or fork, can just run the build without setup.
With a ~/.mvn/jvm.config, you have to tell people to set that up, and figure something out for running it in CI, and also it will apply to all your other Maven projects on that machine as well. So this just seemed like the easier way 😄
In the end though, I'm quite happy with the fact that ErrorProne does work on my 25-ea build, where Spotbugs didn't. It's nice to see all those builds be green, even if they're only experimental ones for future Java versions.
There was a problem hiding this comment.
With a
~/.mvn/jvm.config, you have to tell people to set that up, and figure something out for running it in CI, and also it will apply to all your other Maven projects on that machine as well. So this just seemed like the easier way 😄
I wasn't precise - I'm configuring these under the Maven Wrapper folder (see spring-projects/spring-batch#4807) so .mvn/jvm.config is on git and should be picked up automatically by most users.
(I'm not sure if you run your locally installed Maven and not via ./mnvw, maybe it still looks for that config? I haven't tried it yet)
There was a problem hiding this comment.
Ahhh, I didn't think of that possibility. I don't use the Maven Wrapper myself, so I wasn't aware that you can configure it like that. That actually sounds interesting, though I'm not sure I can fix my muscle memory to type ./mvnw instead of mvn 😅
There was a problem hiding this comment.
I just tried to execute my locally installed mvn, it seems to work 🙂 and it's also documented, unrelated to the wrapper:
Starting with Maven 3.3.1+ you can define JVM configuration via
.mvn/jvm.configfile which means you can define the options for your build on a per project base. This file will become part of your project and will be checked in along with your project. So no need anymore forMAVEN_OPTS,.mavenrcfiles.
There was a problem hiding this comment.
Well then, I guess the answer to your original question is, because I didn't read the documentation 😅
Knowing this, there doesn't seem to be a very good reason for one or the other, apart for personal preference maybe? I think I'll leave it in the pom, at least for now, since it's already working well as it is... Also, I think I like having this stuff close to the rest of the ErrorProne config
There was a problem hiding this comment.
It likely depends on the project, but not forking can improve performance. (Similar to how you use the jar-no-fork goal for the maven-source-plugin.)
There was a problem hiding this comment.
Oh that's good to know. Might change it then after all 😅 Thanks!
There was a problem hiding this comment.
Ahhh, I didn't think of that possibility. I don't use the Maven Wrapper myself, so I wasn't aware that you can configure it like that. That actually sounds interesting, though I'm not sure I can fix my muscle memory to type
./mvnwinstead ofmvn😅
@jqno No need to fix your muscle memory, let the shell do it for you with a small function 😉
https://github.com/merikan/.dotfiles/blob/main/config/sh/functions#L64-L80
There was a problem hiding this comment.
Thanks @merikan, I should have thought of that too! I might steal some other functions from that file as well 😇
| @@ -241,10 +239,38 @@ | |||
| <artifactId>maven-compiler-plugin</artifactId> | |||
| <configuration> | |||
| <showWarnings>true</showWarnings> | |||
| <fork>true</fork> | |||
There was a problem hiding this comment.
It likely depends on the project, but not forking can improve performance. (Similar to how you use the jar-no-fork goal for the maven-source-plugin.)
No description provided.