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

Allow to set config for JDKXRLogger in code rather than config file. #518

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

danfickle
Copy link
Owner

Could be useful, maybe?

@syjer
Copy link
Contributor

syjer commented Jul 23, 2020

for sure nicer :).

By the way, I was wondering if it would be beneficial to move the slf4j logger in the core module (and remove openhtmltopdf-slf4j) and use reflection to autoconfigure it if the slf4j api is present on the classpath.

I think it would simplify the use of the library for the end user (and we still keep the possibility to override it in code).
Additionally we could add the support for log4j2 api too.

@danfickle
Copy link
Owner Author

I'm cautious of using reflection because I'm led to believe it causes all sorts of issues with Java 9 modules (when we upgrade to 11 eventually), osgi modules (#516) and native compilation with Graal VM (which would be nice for serverless). However, if you were to investigate these issues and find them surmountable, I'm not opposed.

P.S. Were you happy with the logging wiki page? Feel free to edit.

@syjer
Copy link
Contributor

syjer commented Jul 23, 2020

you are right about the possible issues, I'll have look :).

By the way, I've already looked to do a native compilation with Graal VM some time ago, unfortunately there was an issue with pdfbox, but it's something that maybe has been fixed. If I'm able to make it run, I'll document it .

Cool about the wiki page, it's something that should I've added.

@danfickle danfickle merged commit fe3d6f7 into open-dev-v1 Jul 24, 2020
@danfickle danfickle deleted the fix_jdkxr_logger branch July 24, 2020 06:49
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.

2 participants