gnodet-bot commented on code in PR #1120: URL: https://github.com/apache/maven-compiler-plugin/pull/1120#discussion_r4086427555
########## 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 +Java 17 to run, but you might need to compile a project with Java 8. +Toolchains are the preferred way to use different JDKs to run Maven and to build the project. -To set this up, refer to the [Guide to Using Toolchains](https://maven.apache.org/guides/mini/guide-using-toolchains.html), -which makes use of the [Maven Toolchains Plugin](https://maven.apache.org/plugins/maven-toolchains-plugin/). +During the build, Maven uses the JDK to perform various steps. +These steps include compiling the Java sources, generating the Javadoc, running unit tests, signing JARs, and more. +Most core Maven plugins execute a JDK tool: `javac`, `javadoc`, `jarsigner`, etc. +A toolchain specifies the path to the JDK where the plugin finds these tools. +It is independent of the JDK that runs Maven itself. -With the maven-toolchains-plugin you configure 1 default JDK toolchain for all related maven-plugins. -Since maven-compiler-plugin 3.6.0 when using with Maven 3.3.1+ it is also possible to give the plugin its own toolchain, -which can be useful in case of different JDK calls per execution block -(e.g. the test sources require a different compiler compared to the main sources). +To set this up, refer to the [Guide to Using Toolchains](https://maven.apache.org/guides/mini/guide-using-toolchains.html) +and the [Maven Toolchains Plugin](https://maven.apache.org/plugins/maven-toolchains-plugin/). + +With the maven-toolchains-plugin, you configure one default JDK toolchain for all related Maven plugins. +Since maven-compiler-plugin 3.6.0, it is also possible assign different plugins different toolchains. Review Comment: 🔴 **Missing verb: `possible assign` → `possible to assign`.** The sentence is missing the infinitive marker `to`. `"it is also possible assign different plugins different toolchains"` is ungrammatical. ```suggestion Since maven-compiler-plugin 3.6.0, it is also possible to assign different plugins different toolchains. ``` ########## src/site/markdown/modules.md: ########## @@ -71,34 +70,34 @@ src └─ org/foo/bar/*.class ``` -The Maven compiler automatically adds `--patch-module`, `--add-modules` and `--add-reads` arguments for compiling the tests. +The Maven compiler automatically adds the `--patch-module`, `--add-modules` and `--add-reads` arguments for compiling the tests. If more `--add-reads` arguments are needed, or if `--add-modules`, `--add-exports` or `--add-opens` arguments are also needed, -then a `module-info-patch.maven` file (syntax described below) can be placed in the `test/java` directory. +then place a `module-info-patch.maven` file (syntax described below) in the `test/java` directory. This Maven file is preferred to a `module-info.java` file in the test directory because the Maven file *completes* the main `module-info.class` (using compiler arguments) instead of *replacing* it. ### Limitation -When using the package hierarchy, problems may occur if the module name is a single name without `.` separator +When using the package hierarchy, problems can occur if the module name is a single name without a `.` separator (for example, `foo` or `bar` but not `foo.bar`) and that name is identical to a package name. -In such case, the hack implemented in the Maven compiler plugin for Maven 3 compatibility +In such a case, the hack implemented in the Maven compiler plugin for Maven 3 compatibility become confused about whether a directory named `foo` represents the module or the package. Review Comment: 🔴 **Subject-verb agreement: `become` → `becomes`.** The subject is `"the hack"` (singular), so the verb must be `becomes`, not `become`. ```suggestion become confused about whether a directory named `foo` represents the module or the package. ``` Wait — the fix is: ```suggestion becomes confused about whether a directory named `foo` represents the module or the package. ``` ########## 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 should be replaced by a `module-info-patch.maven` file in the same directory. - ``` Review Comment: 🔴 **Formatting regression: blank line before code fence removed.** The blank line between the prose paragraph and the ` ``` ` fence was deleted in this PR (same issue that was flagged in `set-compiler-source-and-target.md` in the prior review). Without the blank line, the code block will not render correctly in Doxia/Markdown — the ` ``` ` is treated as inline content of the paragraph. ```suggestion replaced by a `module-info-patch.maven` file in the same directory. ``` ``` -- 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]
