-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fixed logging of java.sql.Date #1378
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! =)
Thank You for your contribution. Can you add some unit tests to prevent regressions? |
-1: this PR is missing a unit test. |
@ppkarwasz @garydgregory done, tested failing before & working after change. Could you please review it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Hikarikun92, it look good to me, thanks.
If there will be no objections I'll merge it in a couple of day.
@ppkarwasz Thanks for the merge! Just one question, the commit with the unit test is gone... is that normal? |
@Hikarikun92, Ooops, thanks for noticing, I must have lost them while locally rebasing. If you can push the tests on the |
@ppkarwasz Done; should I open another PR? 2.x...Hikarikun92:logging-log4j2:2.x |
Thanks, I merged it: 134f4d5 |
@ppkarwasz Thanks! |
Fixed logging of java.sql.Date objects by appending it before Log4J tries to call java.util.Date.toInstant() on it (see #1366).
Checklist
2.x
branch if you are targeting Log4j 2; usemain
otherwise./mvnw verify
succeeds (if it fails due to code formatting issues reported by Spotless, simply runspotless:apply
and retry)src/changelog/.2.x.x
directory