Skip to content

Add new java platform logger#1361

Closed
Sineaggi wants to merge 2 commits intoneo4j:5.0from
Sineaggi:add-java-platform-logger
Closed

Add new java platform logger#1361
Sineaggi wants to merge 2 commits intoneo4j:5.0from
Sineaggi:add-java-platform-logger

Conversation

@Sineaggi
Copy link

@Sineaggi Sineaggi commented Jan 4, 2023

With the java platform logger, we can make the slf4j and jul loggers optional. Users can use implementations of the java platform logging api such as the ones provided by slf4j https://www.slf4j.org/apidocs/org/slf4j/jdk/platform/logging/package-summary.html and log4j https://logging.apache.org/log4j/2.x/log4j-jpl/project-info.html

@michael-simons
Copy link
Contributor

Nice work!

@injectives
Copy link
Contributor

injectives commented Jan 4, 2023

@Sineaggi, thanks for the contribution. 👍

Would you be happy to sign the CLA please? More info here: https://github.com/neo4j/neo4j-java-driver/blob/1.6/CONTRIBUTING.md#want-to-contribute

This is why builds are failing at the moment.

@Sineaggi
Copy link
Author

Sineaggi commented Jan 5, 2023

Would you be happy to sign the CLA please? More info here: https://github.com/neo4j/neo4j-java-driver/blob/1.6/CONTRIBUTING.md#want-to-contribute

I have signed the CLA. Please let me know if there are any issues.

@Sineaggi Sineaggi force-pushed the add-java-platform-logger branch from 953c879 to be9e210 Compare January 5, 2023 00:32
@injectives
Copy link
Contributor

Would you be happy to sign the CLA please? More info here: https://github.com/neo4j/neo4j-java-driver/blob/1.6/CONTRIBUTING.md#want-to-contribute

I have signed the CLA. Please let me know if there are any issues.

Thanks! The whitelist-check works now.

requires io.netty.codec;
requires io.netty.resolver;
requires transitive java.logging;
requires static java.logging;
Copy link
Contributor

Choose a reason for hiding this comment

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

This change triggers compilation failure, caused by the following warning: class java.util.logging.Level in module java.logging is not indirectly exported using requires transitive.

Copy link
Author

Choose a reason for hiding this comment

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

Wow interesting. I tried creating a reproducer locally but wasn't able to.

According to what I've seen online we can make it static and transitive, but I'm not 100% sure. In this case it's true, the logging class is exporting JUL to downstream users (using the logger in the public return types).

Reading through https://nipafx.dev/java-modules-implied-readability/ right now.

Copy link
Author

Choose a reason for hiding this comment

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

Is it still failing for the same reasons?

@injectives
Copy link
Contributor

@Sineaggi, sorry for the delay, but just wanted to let you know that the next major driver version will use the System Logger by default. See the following for more details: #1648

@injectives injectives closed this May 28, 2025
@Sineaggi Sineaggi deleted the add-java-platform-logger branch May 29, 2025 02:57
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