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

Fix LogManager logger call chain #145

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Conversation

gdupontf
Copy link
Contributor

@gdupontf gdupontf commented Apr 7, 2024

What's changed?

This produces the proper method chain to obtain a logger using JUL.

What's your motivation?

Simple bugfix.

Anything in particular you'd like reviewers to focus on?

I didn't change much, but a local build couldn't pass the two license tasks and a few tests just failed on some snakeyaml problems. But the scope of the changes is far from affecting anything there.

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

N/A

Any additional context

Unless you want a specific test that checks that code compiles there's not much to change or add of value in tests.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@timtebeek timtebeek added the bug Something isn't working label Apr 8, 2024
@timtebeek timtebeek self-requested a review April 8, 2024 01:45
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • src/main/java/org/openrewrite/java/logging/log4j/LoggingExceptionConcatenation.java
    • lines 1-0
  • src/test/java/org/openrewrite/java/logging/log4j/LoggingExceptionConcatenationTest.java
    • lines 1-1
    • lines 18-17

@timtebeek
Copy link
Contributor

Thanks a lot! I'll get those suggestions sorted and see this through tomorrow.

@timtebeek timtebeek merged commit 6aa48f9 into openrewrite:main Apr 8, 2024
2 checks passed
@timtebeek
Copy link
Contributor

Great to have you on board! Really helpful to have this issue fixed, and thanks for taking on the extra work too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants