gnodet-bot commented on code in PR #13695: URL: https://github.com/apache/maven/pull/13695#discussion_r4366509237
########## api/maven-api-log/src/main/java/org/apache/maven/api/logging/package-info.java: ########## @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Maven unified logging API — a first-class, SLF4J-free logging facade for + * plugins, extensions, and Maven internals. + * + * <h2>Overview</h2> + * <p> + * This package replaces the older {@code org.apache.maven.api.plugin.Log} + * plugin facade and the SLF4J-backed CLI logger with a single, consistent + * API that works uniformly across mojos, extensions, and Maven's own + * internal components. + * </p> + * + * <h2>Core types</h2> + * <dl> + * <dt>{@link org.apache.maven.api.logging.Logger}</dt> + * <dd>The primary logging interface. Obtained via {@link org.apache.maven.api.logging.Logger#of(Class)} + * or injected by the container. Supports a fluent builder API ({@code atInfo()…log(…)}) + * as well as SLF4J-style convenience methods ({@code info(…)}, {@code warn(…)}, etc.). + * Instances are safe to hold in static or instance fields — context such as + * {@code projectId} and {@code mojoId} is resolved from a per-thread + * {@link org.apache.maven.api.logging.LogSink} at dispatch time.</dd> + * + * <dt>{@link org.apache.maven.api.logging.LogEventBuilder}</dt> + * <dd>Fluent builder for a single log event. Obtained from {@code Logger.atLevel()}. + * Accumulates level, message template, arguments, throwable, and key-value pairs + * before dispatching via {@code log(…)}. When the target level is disabled, returns + * a no-op singleton (zero allocations).</dd> + * + * <dt>{@link org.apache.maven.api.logging.LogEvent}</dt> + * <dd>Immutable record of a single log event. Carries the logger name, level, raw + * message template, arguments (lazy formatting), throwable, and optional + * {@code projectId} / {@code mojoId} context. Passed to {@link org.apache.maven.api.logging.LogSink} + * for consumption.</dd> + * + * <dt>{@link org.apache.maven.api.logging.LogSink}</dt> + * <dd>Consumer of {@link org.apache.maven.api.logging.LogEvent}s. This is the primary + * extension point for embedders and build tools: implement {@code LogSink} and register + * it via {@link org.apache.maven.api.logging.LoggerManager#setLogSink(LogSink)} (or via + * {@link java.util.ServiceLoader} discovery) to redirect all Maven log output to any + * destination — a UI, a structured log store, a test buffer, etc.</dd> + * + * <dt>{@link org.apache.maven.api.logging.LoggerManager}</dt> + * <dd>Central control surface: obtains loggers, installs the active sink, configures + * root and per-logger levels, and tracks the highest severity reached (for + * {@code --fail-on-severity}). Accessed via {@link org.apache.maven.api.logging.LoggerManager#get()}, + * which is set up by Maven before any logging occurs.</dd> + * + * <dt>{@link org.apache.maven.api.logging.LogLevel}</dt> + * <dd>Ordered severity enumeration: {@code TRACE}, {@code DEBUG}, {@code INFO}, + * {@code WARN}, {@code ERROR}.</dd> + * </dl> + * + * <h2>Getting a logger</h2> + * <p> + * The recommended pattern mirrors {@code System.getLogger()}: + * </p> + * <pre>{@code + * private static final Logger log = Logger.of(MyComponent.class); Review Comment: 🟡 **Misleading "recommended pattern" — `Logger.of()` now throws `IllegalStateException` before bootstrap.** The `package-info.java` documents: ```java private static final Logger log = Logger.of(MyComponent.class); ``` as the _"recommended pattern"_ that _"mirrors `System.getLogger()`"_. Unlike `System.getLogger()`, which always returns a valid logger (using a bootstrap logger before the logging system is initialized), `Logger.of()` now throws `IllegalStateException` if called before `LoggerManager.set()`. Static field initializers run at class-load time — any Maven-internal class loaded during the early bootstrap phase (before `DefaultLoggerManager` is constructed) that adopts this pattern will throw. The comparison to `System.getLogger()` sets a false expectation. Either: 1. Add a warning to the pattern: _"This is only safe to call after `LoggerManager` has been initialized"_, or 2. Recommend instance-field initialization instead, or 3. Make `Logger.of()` return a no-op/deferred logger when the manager is null (matching the `System.getLogger()` contract the Javadoc promises) Option 3 is cleanest — it makes the pattern actually safe and the documentation accurate: ```suggestion * The recommended pattern mirrors {@code System.getLogger()}: * </p> * <pre>{@code * private static final Logger log = Logger.of(MyComponent.class); * }</pre> * <p> * {@code Logger.of()} returns a functional logger immediately; the logger * instance is safe to hold in static or instance fields. If the * {@link org.apache.maven.api.logging.LoggerManager} has not been initialized * yet (early-bootstrap use), a no-op logger is returned until the manager is * ready. ``` (Requires `Logger.of()` to return a no-op instead of throwing — see `Logger.java` `@throws` change needed.) Alternatively, if the ISE contract is intentional, document the limitation: ```suggestion * The recommended pattern for use **after Maven bootstrap** mirrors {@code System.getLogger()}: * </p> * <pre>{@code * private static final Logger log = Logger.of(MyComponent.class); * }</pre> * <p> * <strong>Note:</strong> {@code Logger.of()} throws {@link IllegalStateException} if called * before {@link org.apache.maven.api.logging.LoggerManager} is initialized. * Static fields are safe only when the class is guaranteed to be loaded after bootstrap. ``` -- 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]
