yybmion commented on PR #3921: URL: https://github.com/apache/logging-log4j2/pull/3921#issuecomment-3396261253
Hi @vy and @ppkarwasz, Thanks for the feedback! Before I finalize this PR, I'd like to clarify the scope of follow-up work. Based on the discussion, I see two distinct improvement areas: **1. Factory refactoring (removing duplicate code)** > I realize my comments go a bit beyond the immediate scope of this PR, but the section of code below has become a real maintenance headache. It would be great if we could take the opportunity to clean it up once and for all: > > https://github.com/apache/logging-log4j2/blob/8a3fb534f999632d0688a9a204f11b3dd7f59f72/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java#L373-L478 Delegate composite configuration logic in `Factory.getConfiguration(ctx, name, URI)` to the new `getConfiguration(ctx, name, List<URI>)` method. **2. Exception-throwing consistency** >@yybmion, would you be interested in implementing changes necessary to make other ConfigurationFactory::getConfiguration methods throw on failure too? Align all `getConfiguration` methods to throw exceptions instead of returning null ___ I'm currently working on two changes. Should I handle them in separate PRs? -- 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]
