gnodet-bot commented on code in PR #2023:
URL: https://github.com/apache/maven/pull/2023#discussion_r4326849336
##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -649,7 +654,7 @@
<class>
<name>PluginContainer</name>
<version>3.0.0+</version>
- <description>Contains the plugins information for the
project.</description>
+ <description>Returns the plugins information for the
project.</description>
Review Comment:
❌ **Class description uses "Returns" — wrong context.**
`PluginContainer` is a class, not a method. Class descriptions should
describe what the class *is*, not what it *returns*. Change back to a
noun-style description:
```suggestion
<description>Contains the plugins information for the
project.</description>
```
##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -3149,14 +3154,9 @@
<version>4.0.0+</version>
<type>String</type>
<defaultValue>2.0</defaultValue>
- <description><![CDATA[
+ <description>
For a plugin project (packaging is {@code maven-plugin}), the
minimum version of
- Maven required to use the resulting plugin.<br>
- In Maven 2, this was also specifying the minimum version of Maven
required to build a
- project, but this usage is <b>deprecated</b> in Maven 3 and not
checked any more: use
- the <a
href="https://maven.apache.org/enforcer/enforcer-rules/requireMavenVersion.html";>Maven
Enforcer Plugin's
- {@code requireMavenVersion} rule</a> instead.
- ]]>
+ Maven required to use the resulting plugin.
</description>
Review Comment:
❌ **Actionable guidance removed — Enforcer Plugin cross-reference was the
valuable part.**
The Maven 2 historical context ("this also controlled the build-level
minimum…") could reasonably be dropped, but the forward-looking pointer to the
Enforcer Plugin's `requireMavenVersion` rule was the actionable part: it tells
users what to do instead. Without it, users who set `<prerequisites><maven>`
expecting build-level enforcement get no guidance.
Suggested fix (keeping the informative note, dropping the M2 history):
```suggestion
<description>
For a plugin project (packaging is {@code maven-plugin}), the
minimum version of
Maven required to use the resulting plugin. To enforce the
minimum Maven version
required to build a project, use the
<a
href="https://maven.apache.org/enforcer/enforcer-rules/requireMavenVersion.html";>Maven
Enforcer Plugin's
{@code requireMavenVersion} rule</a> instead.
</description>
```
##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -2890,37 +2895,37 @@
<class>
<name>ActivationProperty</name>
<version>4.0.0+</version>
- <description>This is the property specification used to activate a
profile. If the value field
- is empty, then the existence of the named property will activate the
profile, otherwise it
- does a case-sensitive match against the property value as
well.</description>
+ <description>Returns the property specification used to activate a
profile. If the value field
+ is empty, then the existence of the named property will activate the
profile. Otherwise it
+ does a case-sensitive match against the property value.</description>
<fields>
<field>
<name>name</name>
<version>4.0.0+</version>
<type>String</type>
<required>true</required>
- <description>The name of the property to be used to activate a
profile.</description>
+ <description>Returns the name of the property to be used to activate
a profile.</description>
</field>
<field>
<name>value</name>
<version>4.0.0+</version>
<type>String</type>
- <description>The value of the property required to activate a
profile.</description>
+ <description>Returns the value of the property required to activate
a profile.</description>
</field>
</fields>
</class>
<class>
<name>ActivationOS</name>
<version>4.0.0+</version>
- <description>This is an activator which will detect an operating
system's attributes in order
+ <description>Returns an activator which will detect an operating
system's attributes in order
Review Comment:
❌ **Class description uses "Returns" — wrong context.** `ActivationOS` is a
class. Change to a descriptive noun phrase:
```suggestion
<description>This is an activator which will detect an operating
system's attributes in order
```
##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -2890,37 +2895,37 @@
<class>
<name>ActivationProperty</name>
<version>4.0.0+</version>
- <description>This is the property specification used to activate a
profile. If the value field
- is empty, then the existence of the named property will activate the
profile, otherwise it
- does a case-sensitive match against the property value as
well.</description>
+ <description>Returns the property specification used to activate a
profile. If the value field
Review Comment:
❌ **Class description uses "Returns" — wrong context.** `ActivationProperty`
is a class. Change to a descriptive noun phrase:
```suggestion
<description>This is the property specification used to activate a
profile. If the value field
```
##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -2929,30 +2934,30 @@
<version>4.0.0+</version>
<type>String</type>
<description>
- The general family of the OS to be used to activate the profile,
such as
+ Returns the general family of the OS to be used to activate the
profile, such as
{@code windows} or {@code unix}.
</description>
</field>
<field>
<name>arch</name>
<version>4.0.0+</version>
<type>String</type>
- <description>The architecture of the operating system to be used to
activate the
+ <description>Returns the architecture of the operating system to be
used to activate the
profile.</description>
</field>
<field>
<name>version</name>
<version>4.0.0+</version>
<type>String</type>
- <description>The version of the operating system to be used to
activate the
+ <description>Returns the version of the operating system to be used
to activate the
profile.</description>
</field>
</fields>
</class>
<class>
<name>ActivationFile</name>
<version>4.0.0+</version>
- <description>This is the file specification used to activate the
profile. The {@code missing} value
+ <description>Returns the file specification used to activate the
profile. The {@code missing} value
Review Comment:
❌ **Class description uses "Returns" — wrong context.** `ActivationFile` is
a class. Change to a descriptive noun phrase:
```suggestion
<description>This is the file specification used to activate the
profile. The {@code missing}
```
##########
api/maven-api-model/src/main/mdo/maven.mdo:
##########
@@ -768,10 +773,10 @@
<field>
<name>defaultGoal</name>
<version>3.0.0+</version>
- <description>The default goal (or phase in Maven 2) to execute when
none is specified for
+ <description>The default goal to execute when none is specified for
the project. Note that in case of a build with subprojects, only
the default goal of the top-level
- project is relevant, i.e. the default goals of subprojects are
ignored. Since Maven 3,
- multiple goals/phases can be separated by whitespace.</description>
+ project is relevant, i.e. the default goals of subprojects are
ignored.
+ Multiple goals can be separated by whitespace.</description>
Review Comment:
❌ **Accuracy regression — phases are still valid inputs for `defaultGoal`.**
Lifecycle phases (`install`, `verify`, `package`, etc.) remain valid values
for `defaultGoal` in Maven 3 and 4. The original text documented this
explicitly. Removing it is a documentation regression.
Suggested fix:
```suggestion
<description>The default goal or lifecycle phase to execute when
none is specified for
the project. Note that in case of a build with subprojects, only
the default goal of the top-level
project is relevant, i.e. the default goals of subprojects are
ignored.
Multiple goals or phases can be separated by
whitespace.</description>
```
--
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]
