desruisseaux commented on code in PR #1123:
URL:
https://github.com/apache/maven-compiler-plugin/pull/1123#discussion_r4093738732
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -670,7 +679,11 @@ final Charset charset() {
protected String incrementalCompilation;
/**
- * Whether to enable/disable incremental compilation feature.
+ * Whether to enable/disable the change detection that decides when to
recompile the module.
+ * Despite the word "incremental", this does not enable an
+ * incremental compiler in the sense of an IDE. The plugin never compiles
a single changed class
+ * together with the classes that depend on it. It only detects changes
and, depending on the
+ * configuration, recompiles the whole module or only the modified source
files.
Review Comment:
Same comment as for the `incrementalCompilation` option: whether it is an
IDE-style incremental compilation or not depends on which IDE we compare to.
For example, a "build all" in NetBeans delegates to Ant, Maven or Gradle.
The word "never" is a bit strong since more reliable incremental compilation
may be added in the future. I admit that it would not apply to this deprecated
option, but since this option is deprecated, it may not be necessary to add
this paragraph.
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -586,7 +586,16 @@ final Charset charset() {
protected String outputTimestamp;
/**
- * The algorithm to use for selecting which files to compile.
+ * <b>Despite the word "incremental" in the name, this is <em>not</em> an
incremental compiler
+ * in the sense of an IDE.</b> The plugin does not compile a single
changed class and the classes
+ * that depend on it (except when using the {@code modules} algorithm,
which delegates this decision
+ * to the Java compiler). It selects an algorithm used to <i>detect
changes</i> and to decide whether
+ * to recompile the whole module or only some source files. In the default
configuration (no annotation
+ * processors, Java ≥ 23), only the modified source files are
recompiled; a full rebuild is triggered
+ * by a compiler option change, a dependency JAR change, or annotation
processor presence; see the
+ * values and the Default value section below.
+ *
+ * <p>The algorithm to use for selecting which files to compile.
Review Comment:
Remove this line here. It needs to be the first line of this javadoc.
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -586,7 +586,16 @@ final Charset charset() {
protected String outputTimestamp;
/**
- * The algorithm to use for selecting which files to compile.
+ * <b>Despite the word "incremental" in the name, this is <em>not</em> an
incremental compiler
+ * in the sense of an IDE.</b> The plugin does not compile a single
changed class and the classes
+ * that depend on it (except when using the {@code modules} algorithm,
which delegates this decision
+ * to the Java compiler). It selects an algorithm used to <i>detect
changes</i> and to decide whether
+ * to recompile the whole module or only some source files. In the default
configuration (no annotation
+ * processors, Java ≥ 23), only the modified source files are
recompiled; a full rebuild is triggered
+ * by a compiler option change, a dependency JAR change, or annotation
processor presence; see the
+ * values and the Default value section below.
Review Comment:
The AI-generated text is a little bit verbose for saying few (it repeats in
other words the option descriptions). I also have some concerns:
* _"incremental compiler in the sense of an IDE"_ is a vague concept that
depends on the IDE. Some of them just rely on `javac` behaviour, which compares
timestamps like what Maven does. This is why we could claim that Maven Compiler
Plugin 4.x, after the improvements done since 3.x, has an IDE-style incremental
compilation, while actually it depends which IDE we compare to.
* _"except when using the `modules` algorithm, which delegates this decision
to the Java compiler"_ gives the impression that `javac` checks dependencies in
such case, which is not accurate. What AI claims to be a clarification actually
brings confusion. The `javac` behaviour in such case is actually quite similar
to the Maven behaviour. This sentence consumes space for saying something that
change almost nothing for the user regarding incremental compilation.
* We do not implement yet an algorithm that check dependencies, but it has
been requested by users and may be added in the future. It would be a new
keyword in the list of recognized algorithms. Therefore, a wording that feel
too final may be misleading.
I propose a shorter and more nuanced paragraph like below (minus my
potentially broken English). Please feel free to reword:
> The incremental compilation controlled by this option is not yet as
reliable as the incremental compilation provided by some IDEs, but provides an
approximation based on the timestamps of source files. Many algorithms (listed
below) can be combined for tuning the trade-off between overhead and
reliability. The current algorithms detect only direct changes (recompiling
only modified source file), but a future version of this plugin may add an
option for tracking classes that depend on a modified class, as done by some
IDEs.
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -670,7 +679,11 @@ final Charset charset() {
protected String incrementalCompilation;
/**
- * Whether to enable/disable incremental compilation feature.
+ * Whether to enable/disable the change detection that decides when to
recompile the module.
Review Comment:
Replace "module" by "sub-project".
--
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]
