gnodet commented on PR #12694: URL: https://github.com/apache/maven/pull/12694#issuecomment-6360137849
Thanks for the review, Romain. Your inline comments have been addressed in the thread replies, but let me respond to the overall direction concern here. The `LogEvent` API currently mixes two things: 1. **Log data** — `level()`, `message()`, `loggerName()`, `threadId()`, `instant()`, `stackTrace()` — these are intrinsic to the event. 2. **Rendering artefact** — `formattedMessage()` — this is a pre-rendered console line, which you rightly point out doesn't belong in a data structure. I agree with you on `formattedMessage()`: it leaks a rendering concern into what should be a rendering-agnostic interface. I'm removing it. The consumer (`SimpleBuildEventListener` and the chain in PR #13180) will carry its own formatter — either a `LogEventFormatter` utility class or a `Function<LogEvent, String>` injected at construction. I'll coordinate with #13180 before finalising that. On the other comments: - **`message()` vs template+args**: JUL eagerly substitutes so no template is available; exposing it would create an inconsistent API. Keeping `message()` as the substituted string, `formattedMessage()` is gone. - **`Throwable` vs `String`**: `@Immutable` + serialisation concern argues for `String`. I'm adding a default-null `throwable()` as an opt-in for consumers that need type-based filtering, alongside `stackTrace()` which stays for the general case. - **Custom `LogManager`**: Adding the guard in `LookupInvoker` — check `LogManager.getLogManager().getClass() == LogManager.class` before touching root handlers. - **Thread name**: Adding `threadName()` populated at capture time. I'll address these in the next commit. The overall shape of the API (`LogEvent` as a data record of what was logged, not how it was rendered) is the goal — thanks for pushing on that. -- 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]
