-
Notifications
You must be signed in to change notification settings - Fork 745
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
#1185 add a note about Lombok to the installation instructions #1186
Conversation
docs/installation.md
Outdated
All of the above instructions use the `javac` option `-processorpath` which has | ||
side-effect of causing `javac` to no longer scan the compile classpath for | ||
annotation processors. If you are using other annotation processors in addition | ||
to ErrorProne, such as Lombok, then you will need to add their JARs to your |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…or Dagger, AutoValue, Mapstruct, Immutables, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course.
Are you asking me to change the PR to list these other examples?
I don't think that adds a lot, but I am happy to do so if you like.
Are you happy with listing those 5 examples? It seems a bit overkill to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated this to remove the Lombok example, if it is a sticking point
annotation processors. If you are using other annotation processors in addition | ||
to ErrorProne, such as Lombok, then you will need to add their JARs to your | ||
`-processorpath` argument. The mechanics of this will vary according to the build | ||
tool you are using. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…too bad this does not list them 😉
How about adding a comment to the snippets above saying something like "insert your other annotation processors here"?
(though that this applies mainly to Maven; it gained only recently the ability to use the processor path, and almost nobody updated their build, and most importantly their knowledge)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point
I did similar to that in the adjacent PR #1187
I will add the same change to Maven above.
I'll have a go at the others, but Bazel and Gradle look like closed black-boxes to me that don't admit such a change at present
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maven gained only recently the ability to use the processor path, and almost nobody updated their build, and most importantly their knowledge
Why use "-processorpath" instead of adding these things to the main classpath in "provided" scope? It seems to me that the latter is more robust and much easier when multiple annotation processors (or custom ErrorProne rules) are in use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ErrorProne is integrated into Bazel, it's not something you enable/disable, so there's nothing special to be said.
On Gradle side, recent versions emit a warning if you don't use the processor path (annotationProcessor
configuration), and Gradle 5.0 will require you to use the processor path. I'll add a note to the plugin's README if I have feedback that it might be necessary, but my impression is that most Gradle users are used to that annotationProcessor
configuration (many annotation processors' documentation –even Lombok's one– tell users to use it, or the equivalent apt
configuration with my net.ltgt.apt
plugin)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use "-processorpath" instead of adding these things to the main classpath in "provided" scope? It seems to me that the latter is more robust and much easier when multiple annotation processors (or custom ErrorProne rules) are in use.
ErrorProne is a plugin (-Xplugin
), and those are only loaded from the processor path, not the compilation classpath.
Wrt other usages of annotation processors, without ErrorProne in loop, having too separate classpaths means you can have separate dependencies, or put differently, that annotation processors won't impose their own dependencies on you. For example, if you use Guava and target Java 7, you won't use a version past 20.0 (or so, from memory); or if you target Android you'll use a special version targeting Android; you don't want your annotation processors (such as Dagger or AutoValue) to impose a different or more recent version of Guava. Putting them in the processor path helps segregate those dependency trees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that makes sense
Addresses the question I raised in #1185: how to use Lombok with ErrorProne