gnodet-bot commented on code in PR #13277:
URL: https://github.com/apache/maven/pull/13277#discussion_r4115146151
##########
apache-maven/src/assembly/maven/bin/mvn:
##########
@@ -499,7 +501,13 @@ if $cygwin ; then
fi
handle_args() {
+ _skip_next=false
while [ $# -gt 0 ]; do
+ if $_skip_next ; then
+ _skip_next=false
+ shift
+ continue
+ fi
Review Comment:
⚠️ **Not addressed from previous review:** `_skip_next` is initialized to
`false` (line 504) and checked (line 506), but is never set to `true` anywhere
in `handle_args()`. The `--clapp` handler at line 550 uses `shift` directly to
consume its argument, so the iteration naturally advances past the tool name.
This entire block is dead code.
```suggestion
while [ $# -gt 0 ]; do
```
##########
apache-maven/src/assembly/maven/bin/mvn:
##########
@@ -546,6 +597,15 @@ cmd="\"$JAVACMD\" \
\"-Dlibrary.jline.path=$JLINE_NATIVE_PATH\" \
\"-Dmaven.multiModuleProjectDirectory=$MAVEN_PROJECTBASEDIR\""
+# When a CLAPP is active, inject the tool name and main class as system
properties
+# so that MavenClappCling (and m2.conf) can pick them up.
Review Comment:
💡 **Stale comment:** Since the m2.conf `optionally` directive was removed
(fixing finding #1), m2.conf no longer picks up CLAPP properties. Only
`MavenClappCling` reads these system properties now.
```suggestion
# so that MavenClappCling can pick them up.
```
--
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]
