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

Use JBoss Commons Logging Implementation - Amazon Alexa #10655

Merged
merged 1 commit into from
Jul 15, 2020

Conversation

oztimpower
Copy link
Contributor

Enforce the use of the JBoss Apache Commons Logging Implementation by default for use with native image compilation of Amazon Alexa #10589, as explained in the documentation update #10474

The Amazon ASK SDK v2 uses Commons Logging, and an SL4J implementation, which is excluded by the Quarkus Amazon Alexa extension.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Question: should we add the SLF4J connector too?

@oztimpower
Copy link
Contributor Author

I would probably need help on what the JBoss Logging SL4J-impl is, as I had looked in the Maven Repo and couldn't find it (hence didn't add).

My understanding is that SL4J will do "no-op" logging if an implementation is not found, though I suspect there may be some order preference being used, just like JBoss Logging does.

@dmlloyd
Copy link
Member

dmlloyd commented Jul 13, 2020

All of these adapters should be added to the core IMO. It was just an error on my part that they were left off. But probably it should be commons-logging-jboss-logmanager, slf4j-jboss-logmanager, log4j-jboss-logmanager, and log4j2-jboss-logmanager.

@gsmet
Copy link
Member

gsmet commented Jul 13, 2020

@gsmet
Copy link
Member

gsmet commented Jul 13, 2020

But yeah, either we add them when we exclude the implementation or we add them all to the core artifacts.

@oztimpower
Copy link
Contributor Author

oztimpower commented Jul 15, 2020

Thanks @dmlloyd @gsmet for the SL4J details, apologies for missing it in the docs

I've updated the pom to include both, however I probably think it makes sense also to have this driven from the core.

@gsmet gsmet added this to the 1.7.0 - master milestone Jul 15, 2020
@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 15, 2020
@gsmet gsmet merged commit 68861e1 into quarkusio:master Jul 15, 2020
@gsmet
Copy link
Member

gsmet commented Jul 15, 2020

Thanks!

@gsmet gsmet modified the milestones: 1.7.0 - master, 1.6.1.Final Jul 16, 2020
@oztimpower oztimpower deleted the ext/alexa-logging branch March 5, 2021 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants