vy commented on code in PR #3789: URL: https://github.com/apache/logging-log4j2/pull/3789#discussion_r2182036233
########## log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NamedPattern.java: ########## Review Comment: Disclosing legacy patterns in the binary footprint will make it impossible to deprecate the legacy support in the future. I'd appreciate it if you can update this class as follows: 1. Rename this class to `DatePattern` 2. Add minimal, to-the-point JavaDoc to the class definition. Please stick to [the one-sentence-per-line convention](https://asciidoctor.org/docs/asciidoc-recommended-practices/#one-sentence-per-line) we adhere to in our docs. 3. Create a `private static final boolean COMPAT = InstantPatternFormatter.LEGACY_FORMATTERS_ENABLED;` field and copy the entire documentation from `DatePatternConverter` to here in Javadoc (i.e., in between `/** ... */`) 4. Make enum ctor to receive a single `String` property (call it `pattern` in the class field): ``` ... ABSOLUTE_NANOS("HH:mm:ss," + (COMPAT ? "nnnnnnnnn" : "SSSSSSSSS")), ... ``` 5. Add a `public static String decode(final String pattern)` method copy the associated JavaDoc from `DatePatternConverter`. Don't build the logic on catching `IllegalArgumentException`; use a `for`-loop over `values()`, or `Arrays.stream(values()).findFirst(...).map(...).orElse(pattern)`, etc. 6. Remove `DatePatternConverter#decodeNamedPattern(String)` 7. Fix `DatePatternConverter` and `DatePatternConverterTestBase.java` 8. Bump the `@Version` in `package-info.java` to `2.26.0` -- 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: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
