gnodet-bot commented on code in PR #11102:
URL: https://github.com/apache/maven/pull/11102#discussion_r4269394301
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java:
##########
@@ -616,4 +617,20 @@ protected void logSummary(
logSummary(context, child, references, indent);
}
}
+
+ @Override
+ protected void preCommands(final MavenContext context) throws Exception {
+ super.preCommands(context);
+
+ try {
+ final long pid = ProcessHandle.current().pid();
+ final String version =
String.valueOf(getClass().getPackage().getImplementationVersion());
+ final Path workDir = context.cwd.get();
+ final Path execRoot =
context.invokerRequest.rootDirectory().orElse(context.invokerRequest.topDirectory());
+ ProcessRuns.install(version, workDir, execRoot);
+ context.closeables.add(() -> ProcessRuns.uninstall(pid));
+ } catch (final Throwable ignored) {
Review Comment:
⚠️ **`catch (Throwable)` is too broad here.** This silently swallows
JVM-fatal errors like `OutOfMemoryError` and `StackOverflowError`, making
catastrophic failures invisible. Since `ProcessRuns.install()` already handles
`IOException | RuntimeException` internally, and the remaining calls
(`ProcessHandle.current().pid()`,
`getClass().getPackage().getImplementationVersion()`, context accessors) only
throw `RuntimeException` subclasses — narrowing to `catch (Exception)`
preserves the best-effort semantics without muffling fatal errors.
```suggestion
} catch (final Exception ignored) {
```
##########
api/maven-api-cli/src/main/java/org/apache/maven/api/cli/Options.java:
##########
@@ -220,4 +220,14 @@ default void warnAboutDeprecatedOptions(@Nonnull
ParserRequest request, @Nonnull
* @param printWriter the string consumer to use for output
*/
void displayHelp(@Nonnull ParserRequest request, @Nonnull Consumer<String>
printWriter);
+
+ /**
+ * Indicates whether to list running Maven processes and exit.
+ * Implementations that don't support this option can leave it empty.
+ * @since 4.1.0
+ */
+ @Nonnull
+ default Optional<Boolean> listProcesses() {
Review Comment:
💡 **Naming convention:** All existing `Options` accessors use noun/adjective
style — `help()`, `offline()`, `quiet()`, `verbose()`, `projects()`, `goals()`,
`threads()`, `userProperties()`, etc. `listProcesses()` is the only imperative
verb-command accessor in the interface. Per the accessor-style convention
applied consistently across `Options` and `MavenOptions`, consider renaming to
`processes()`.
```suggestion
default Optional<Boolean> processes() {
```
--
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]
