-
Notifications
You must be signed in to change notification settings - Fork 83
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
Migrate AuditorAware to an Optional<> for spring boot 1 to 2 migration #614
Conversation
Jenson3210
commented
Oct 30, 2024
•
edited by timtebeek
Loading
edited by timtebeek
- This closes spring-data-commons changed AuditorAware from 1.x to 2.x #613
@timtebeek I've added 4 Thanks! |
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Outdated
Show resolved
Hide resolved
@timtebeek Can we also go over the changes of the bot once? |
Hi yes unfortunately the .patch file is correct, but once converted into comments by https://github.com/googleapis/code-suggester it's sometimes a little off when crossing multiple lines. I have yet to fix that external tool, or write my own. You can also run the recipes directly if you prefer, possibly through the Moderne CLI to make it most convenient: |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Okay. Makes sense. On my private pc I do not have moderne cli set up yet as I do not have the access token etc. Would that be possible for the open source projects (like open rewrite itself)? |
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Yes for OSS you would not even need a Moderne license; Just a Moderne public instance personal access token to download recipes and LSTs. |
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Show resolved
Hide resolved
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.
Great to see how much of this you've covered already; I've applied some fixes already and left a few comments for other work left to do. Didn't quite finish the last failing test, but we can revisit that once the other items are crossed off.
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptionalTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/spring/data/MigrateAuditorAwareToOptional.java
Outdated
Show resolved
Hide resolved
Hi @timtebeek , I've pushed some comments you made in the past 3 rounds of rewiew.
Strangly, both are in the don't -> lambda part. So it looks like the lambda in the test code can't be parsed. |
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.
Great to see! The last remaining failing tests were using a method signature from 2.x as opposed to 1.x, so changing the parser classpath for those tests while still asserting no change is made helped get those to pass.
There might be some scenarios not covered like a class defined in a separate file outside of an immediate return, but we can tackle those separately if they even occur in your wider scan there.
Thanks again!