substrait-io / substrait-java

Apache License 2.0
72 stars 70 forks source link

feat: allow deployment time selection of logging framework #243 #244

Closed bestbeforetoday closed 2 months ago

bestbeforetoday commented 2 months ago

Depend on slf4j-api instead of the slf4j-jdk14 logging framework. This allows consumers to select their preferred logging framework at deployment time by adding an appropriate SLF4J provider to their runtime classpath.

If no SLF4J provider is available at runtime, no logging output will be produced. The code still functions as expected.

The isthmus CLI command continues to log using java.util.logging by including the slf4j-jdk14 provider in its runtime classpath.

CLAassistant commented 2 months ago

CLA assistant check
All committers have signed the CLA.

vbarua commented 2 months ago

@bestbeforetoday changes look reasonable to me. You'll need to sign the CLA before we can get this merged in though.

bestbeforetoday commented 2 months ago

@vbarua Thank you for the quick feedback. I've made another pass to refine the change. I've still left the PR in draft as I need to get a review of the CLA from my employer before I can sign off on it. I'll mark it ready for review once I have that done. In the meantime, any feedback or change suggestions are very welcome.

bestbeforetoday commented 2 months ago

@vbarua I have rebased on the latest commit and this should be ready for review now.