gnodet-bot commented on code in PR #1120: URL: https://github.com/apache/maven-compiler-plugin/pull/1120#discussion_r4083248269
########## src/site/markdown/examples/annotation-processor.md: ########## @@ -19,24 +19,24 @@ under the License. # Annotation processors -[Annotation processing](https://docs.oracle.com/en/java/javase/23/docs/specs/man/javac.html#annotation-processing) is used to let the compiler generate source code based on annotations. +[Annotation processing](https://docs.oracle.com/en/java/javase/23/docs/specs/man/javac.html#annotation-processing) generates source code based on annotations. For example, the [Hibernate Processor](https://hibernate.org/orm/processor/) provides an annotation processor to generate the JPA metamodel. ## Recommended way to activate annotation processing -Up to JDK 23, the compiler automatically scanned the classpath for annotation processors and executed all found by default. -For security reasons, this got disabled by default since JDK 23 and annotation processing needs to be activated explicitly. -The recommended way for this is to list all desired processors using either the `<annotationProcessors>` plugin configuration -or, when using Maven 4 and Maven Compiler Plugin version 4.x, by declaring the processors as dependencies of type `processor`. `classpath-processor` or `modular-processor`. -Only those processors will get executed by the compiler. +Through JDK 23, the compiler automatically scans the classpath for annotation processors. It executed all processors found by default. +For security reasons, this is disabled by default in JDK 24 and later. You must activate annotation processing explicitly. Review Comment: ⚠️ **Factual error: wrong JDK version boundary.** The Oracle JDK 23 release notes (JDK-8321314) explicitly state: *"As of JDK 23, annotation processing is only run with some explicit configuration..."* The current live Maven docs also say "disabled by default since JDK 23". Changing this to "JDK 24 and later" contradicts the official JDK 23 release notes — the change happened in JDK 23, not JDK 24. Also, this block has a tense inconsistency: "the compiler automatically **scans**" (present) immediately followed by "It **executed**" (past). Both sentences describe the same historical pre-JDK-23 behavior and should use the same tense. ```suggestion Through JDK 22, the compiler automatically scanned the classpath for annotation processors and executed all processors found by default. For security reasons, this is disabled by default since JDK 23. You must activate annotation processing explicitly. ``` ########## src/site/markdown/modules.md: ########## @@ -53,9 +53,8 @@ such as `--add-reads` in the `<testCompilerArgs>` element of the plugin configur ## Maven 4 with package hierarchy Maven 4 allows the same directory layout as Maven 3. -However, the `module-info.java` file in the test directory *should* be +However, the `module-info.java` file in the test directory must be Review Comment: ⚠️ **Semantic overstatement: `should` → `must`.** The preceding sentence says *"Maven 4 still supports this approach for compatibility reasons. However, it is deprecated."* A deprecated-but-still-supported feature is not required to be replaced — it is *recommended* to replace it. Changing `should be` to `must be` falsely implies it is required (i.e., not replacing it breaks the build), which contradicts the compatibility guarantee stated in the same paragraph. ```suggestion However, the `module-info.java` file in the test directory should be ``` ########## src/site/markdown/examples/compile-using-different-jdk.md: ########## @@ -21,29 +21,33 @@ under the License. ## Using Maven Toolchains -The preferable way to use a different JDK is to use the toolchains mechanism. -During the build of a project, Maven, without toolchains, will use the JDK to perform various steps, -like compiling the Java sources, generate the Javadoc, run unit tests or sign JARs. -Each of those plugins need a tool of the JDK to operate: `javac`, `javadoc`, `jarsigner`, etc. -A toolchain is a way to specify the path to the JDK to use for all of those plugins in a centralized manner, -independent from the one running Maven itself. +Maven is itself a Java application running in a JDK. +By default the same JDK that runs Maven builds the code and runs the tests. +However, sometimes you need different JDKs. For instance, recent versions of Maven require Review Comment: 🧹 **Trailing whitespace.** There is a trailing space at the end of this line (`Maven require ⎵`). Remove it. ```suggestion However, sometimes you need different JDKs. For instance, recent versions of Maven require ``` ########## src/site/markdown/examples/set-compiler-source-and-target.md: ########## @@ -42,9 +42,7 @@ you can either add the two following properties, which are the default property </project> ``` -or configure the plugin directly: - -```xml +or configure the plugin directly:```xml Review Comment: 🔴 **Formatting regression: prose merged with fenced code block.** The blank line between the prose and the `\`\`\`xml` code fence was removed. In Markdown, a fenced code block must be preceded by a blank line when it follows a paragraph. Without the blank line, the ` ```xml` is rendered as inline content of the paragraph, not as a code block. The XML snippet below will not render as a code block. ```suggestion or configure the plugin directly: ```xml ``` -- 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]
