Closed Riyazul555 closed 5 months ago
@timtebeek please check this PR thanks
Thanks for getting this started @Riyazul555 ; I've added a unit test to highlight what I think should happen here, based on what @jeffreye had described in #159 . Would you want to fix those arguments still?
No thanks @timtebeek You can just draft it
Thank you both. I just looked at the PR, we should also consider log4j2 and other markers( if any) as well.
Best, Jeffrey
From: Md Riyazul Islam @.> Sent: Saturday, June 29, 2024 6:44:06 AM To: openrewrite/rewrite-logging-frameworks @.> Cc: Jeffrey Ye @.>; Mention @.> Subject: Re: [openrewrite/rewrite-logging-frameworks] ParameterizedLogging recipe fixed (PR #161)
No thanks @timtebeekhttps://github.com/timtebeek You can just draft it
— Reply to this email directly, view it on GitHubhttps://github.com/openrewrite/rewrite-logging-frameworks/pull/161#issuecomment-2198099132, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AANIHUGNK4OX5ZRHOZ7YQYLZJ2FXNAVCNFSM6AAAAABKDCTXWSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJYGA4TSMJTGI. You are receiving this because you were mentioned.Message ID: @.***>
@timtebeek can you provide me more what do you need in this PR ? Thanks
Sure; if you run the new test I added you'll find it produces an unexpected result
Unexpected result in "Test.java":
diff --git a/Test.java b/Test.java
index d494749..01aece4 100644
--- a/Test.java
+++ b/Test.java
@@ -3,6 +3,6 @@
class Test {
static void method(Logger logger, Marker marker, String name) {
- logger.info(marker, "Hello {}, nice to meet you {}", name, name);
+ logger.info("Hello {}, nice to meet you {}", marker, name, name);
}
}
You'll notice that the marker is moved from being the first argument, to being the second, which changes the behavior. We'll want to keep the marker as the first argument.
Superseeded by
What's changed?
What's your motivation?
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
Checklist