gnodet-bot commented on code in PR #1143:
URL:
https://github.com/apache/maven-compiler-plugin/pull/1143#discussion_r4227418300
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -642,12 +642,17 @@ final Charset charset() {
*
* <h4>Default value</h4>
* The default value depends on the context.
- * If there is no annotation processor, then the default is {@code
"options,dependencies,sources"}.
+ * If there is no annotation processor, then the default is {@code
"options,dependencies,sources,rebuild-on-change"}.
* It means that a full rebuild will be done if the compiler options or
the dependencies changed,
- * or if a source file has been deleted. Otherwise, only the modified
source files will be recompiled.
+ * or if a source file has been deleted, or if any source file has been
modified.
+ * This ensures correctness: if a method signature changes in {@code
Foo.java}, all classes that depend
+ * on {@code Foo} will be recompiled together with it, preventing stale
{@code .class} files that could
+ * cause {@link NoSuchMethodError} at runtime.
+ * Users who prefer faster (but potentially unsafe) incremental builds can
set
+ * {@code "options,dependencies,sources"} explicitly.
*
* <p>If an annotation processor is present (e.g., {@link #proc} set to a
value other than {@code "none"}),
- * then the default value is same as above with the addition of {@code
"rebuild-on-add,rebuild-on-change"}.
+ * then the default value is same as above with the addition of {@code
"rebuild-on-add"}.
Review Comment:
The Javadoc currently says the annotation-processor default is "same as
above with the addition of `rebuild-on-add`", but after PR #1136 merged,
`amendincrementalCompilation()` has two branches:
- **Uncertain case** (Java < 23, proc unset, no processor path): base +
`REBUILD_ON_ADD` + `REBUILD_ON_CHANGE` → the sentence is correct for this branch
- **Confirmed case** (`proc` set explicitly to non-`none`, or Java ≥ 23 with
explicit processor paths): `aspects.clear()` → `NONE` — incremental compilation
is **entirely disabled**, which is not described here at all
Both branches should be documented, e.g.:
> When the presence of an annotation processor is uncertain (Java < 23, proc
unset), the default is
`options,dependencies,sources,rebuild-on-change,rebuild-on-add`. When a
processor is confirmed (proc explicitly set, or Java ≥ 23 with processor
paths), incremental compilation is disabled entirely (`none`).
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -642,12 +642,17 @@ final Charset charset() {
*
* <h4>Default value</h4>
* The default value depends on the context.
- * If there is no annotation processor, then the default is {@code
"options,dependencies,sources"}.
+ * If there is no annotation processor, then the default is {@code
"options,dependencies,sources,rebuild-on-change"}.
* It means that a full rebuild will be done if the compiler options or
the dependencies changed,
- * or if a source file has been deleted. Otherwise, only the modified
source files will be recompiled.
+ * or if a source file has been deleted, or if any source file has been
modified.
+ * This ensures correctness: if a method signature changes in {@code
Foo.java}, all classes that depend
+ * on {@code Foo} will be recompiled together with it, preventing stale
{@code .class} files that could
+ * cause {@link NoSuchMethodError} at runtime.
+ * Users who prefer faster (but potentially unsafe) incremental builds can
set
+ * {@code "options,dependencies,sources"} explicitly.
*
* <p>If an annotation processor is present (e.g., {@link #proc} set to a
value other than {@code "none"}),
- * then the default value is same as above with the addition of {@code
"rebuild-on-add,rebuild-on-change"}.
+ * then the default value is same as above with the addition of {@code
"rebuild-on-add"}.
Review Comment:
The Javadoc currently says the annotation-processor default is "same as
above with the addition of `rebuild-on-add`", but after PR #1136 merged,
`amendincrementalCompilation()` has two branches:
- **Uncertain case** (Java < 23, proc unset, no processor path): base +
`REBUILD_ON_ADD` + `REBUILD_ON_CHANGE` → the sentence is correct for this branch
- **Confirmed case** (`proc` set explicitly to non-`none`, or Java ≥ 23 with
explicit processor paths): `aspects.clear()` → `NONE` — incremental compilation
is **entirely disabled**, which is not described here at all
Both branches should be documented, e.g.:
> When the presence of an annotation processor is uncertain (Java < 23, proc
unset), the default is
`options,dependencies,sources,rebuild-on-change,rebuild-on-add`. When a
processor is confirmed (proc explicitly set, or Java ≥ 23 with processor
paths), incremental compilation is disabled entirely (`none`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
