Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reworked ability to disable initializing Slf4J #9185

Merged
merged 4 commits into from
May 18, 2023

Conversation

altro3
Copy link
Contributor

@altro3 altro3 commented Apr 30, 2023

Unfortunately, the previous solution didn't work because the static fields were still initialized regardless of usage.

This solution is much more efficient and works 100% because I tested it several times :). Now, if necessary, the logEnabled flag is set in the constructor and the logger is not static, but is created through the factory only if necessary. Otherwise, a dummy logger is created.

Thus, the code remains as it was and we have the opportunity to turn off the initialization of slf4j in micronaut-openapi.

This solution also corrects the names of loggers when inheriting. Now the child and parent methods use a single logger with the name of the child class (this cannot be done when you have a static logger)

@graemerocher graemerocher marked this pull request as draft May 1, 2023 09:22
@graemerocher
Copy link
Contributor

The PR includes several breaking API changes which will have to be corrected first plus the tests are failing

…ble_log_2

# Conflicts:
#	core/src/main/java/io/micronaut/core/io/scan/DefaultClassPathResourceLoader.java
#	inject/src/main/java/io/micronaut/context/env/AbstractPropertySourceLoader.java
#	inject/src/main/java/io/micronaut/context/env/PropertySourcePropertyResolver.java
@altro3
Copy link
Contributor Author

altro3 commented May 1, 2023

@graemerocher Need to return methods isLogEnabled() and setLogEnabled(boolean logEnabled) ?

@altro3 altro3 force-pushed the fix_disable_log_2 branch 2 times, most recently from 4ab68b2 to 1ec5466 Compare May 1, 2023 12:18
@altro3
Copy link
Contributor Author

altro3 commented May 1, 2023

@graemerocher tests fixed. Also fixed several checkstyle errors, doclint errors and fixed 2 tests for windows

Copy link
Contributor

@graemerocher graemerocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also needs test coverage for the case this is trying to fix

@altro3 altro3 requested a review from graemerocher May 2, 2023 16:25
@graemerocher graemerocher marked this pull request as ready for review May 3, 2023 08:02
@graemerocher graemerocher added this to the 3.9.2 milestone May 3, 2023
@graemerocher graemerocher requested a review from sdelamo May 3, 2023 08:02
@sdelamo sdelamo merged commit 84d64a5 into micronaut-projects:3.9.x May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants