ramanathan1504 commented on PR #4128: URL: https://github.com/apache/logging-log4j2/pull/4128#issuecomment-4533194976
@ashr123 Thanks for the detailed explanation! I've been analyzing the whole picture based on our discussion, and I want to clarify my stance. We have a classic architectural trade-off here: On one hand, avoiding a new public class (perhaps by nesting it inside DatePatternConverter) keeps our public API surface small. On the other hand, keeping NamedInstantPattern as a brand-new, isolated enum keeps the code very clean, avoids bloating the old class, and seamlessly handles the 2.x compatibility you mentioned. For me, I am completely fine with either approach. Both have valid merits. Since this deals with the core public API structure, let's just wait for the core maintainers (like @ppkarwasz or @vy ) to weigh in. Whichever direction they align with—whether keeping the new class or nesting it in the old one—I will happily support it. Once they give the green light on the final structure, I'll gladly finish up the review and merge this. Thanks again for the great discussion and all your work on this! -- 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]
