Skip to content

Conversation

@woj-tek
Copy link
Contributor

@woj-tek woj-tek commented Jul 8, 2024

What's changed?

Added conversion from JUL to slf4j calls that have Throwable as parameter included

What's your motivation?

Continuation of #155 and PR: #160

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

Anyone you would like to review specifically?

@timtebeek

Have you considered any alternatives or workarounds?

Any additional context

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

Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Great to see! Love how quickly these cases can now be converted. Called out two suspected copy-paste issues, apart from that I think we're good! :)

@timtebeek timtebeek merged commit 3902a76 into openrewrite:main Jul 8, 2024
@timtebeek timtebeek added the recipe Recipe Requested label Jul 8, 2024
@woj-tek
Copy link
Contributor Author

woj-tek commented Jul 9, 2024

Thanks for spotting the errors (and yes, there were some copy-pasting, guilty as charged :D).

Btw. this seemed simple and I think quite common case - take a method and rearrange it's arguments (and optionally change it's name/package). Do you fathom it could be feasible in the future to do it via YAML recipes? It felt like a lot of boilerplate copy-paste that would make it good candidate for such YAML.

@timtebeek timtebeek deleted the jul-to-slf4j-calls-with-throwable branch July 9, 2024 10:33
@timtebeek
Copy link
Member

Thanks for spotting the errors (and yes, there were some copy-pasting, guilty as charged :D).

All good! Nice to have this mostly sorted.

Btw. this seemed simple and I think quite common case - take a method and rearrange it's arguments (and optionally change it's name/package). Do you fathom it could be feasible in the future to do it via YAML recipes? It felt like a lot of boilerplate copy-paste that would make it good candidate for such YAML.

Yes we have configurable building block recipes along those lines like

  • org.openrewrite.java.ReorderMethodArguments
  • org.openrewrite.java.DeleteMethodArgument
  • org.openrewrite.java.ChangeMethodName
  • org.openrewrite.java.ChangePackage

You could compose these together to make similar changes, although some parts, like calling a chain of methods, would not fit that model very well. It would also be more lines of code, to compose each of those recipe steps, multiplied by the number of logging methods to convert. And then of course there's no compiler checking that there's no misspellings or anything.

In this case I think the Refaster style recipes are the most natural fit for the problem at hand, even if there's a bit of boilerplate in terms of the documentation annotations. Hope you agree!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

recipe Recipe Requested

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Recipe for migrating from JUL to slf4j? With support for Logger.log(Level,String) calls?

2 participants