-
Notifications
You must be signed in to change notification settings - Fork 73
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
Joda time to Java time: Add few more templates for migration #594
Conversation
src/main/java/org/openrewrite/java/migrate/joda/templates/AbstractInstantTemplates.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/joda/templates/DateTimeFormatterTemplates.java
Outdated
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/migrate/joda/templates/InstantTemplates.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/migrate/joda/JodaTimeVisitorTest.java
Outdated
Show resolved
Hide resolved
import java.time.Instant; | ||
import java.time.ZoneId; | ||
import java.time.ZoneOffset; | ||
import java.time.ZonedDateTime; | ||
import java.time.format.DateTimeFormatter; | ||
|
||
class A { | ||
public void foo() { | ||
ZonedDateTime.parse("2024-10-25T15:45:00", DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | ||
ZonedDateTime.parse("2024-10-25T15:45:00", DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")).toInstant().toEpochMilli(); | ||
ZonedDateTime.ofInstant(Instant.ofEpochMilli(1234567890L), ZoneId.systemDefault()).format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | ||
ZonedDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | ||
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss").withZone(ZoneOffset.UTC); | ||
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss").withZone(ZoneOffset.UTC); |
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.
import java.time.Instant; | |
import java.time.ZoneId; | |
import java.time.ZoneOffset; | |
import java.time.ZonedDateTime; | |
import java.time.format.DateTimeFormatter; | |
class A { | |
public void foo() { | |
ZonedDateTime.parse("2024-10-25T15:45:00", DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | |
ZonedDateTime.parse("2024-10-25T15:45:00", DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")).toInstant().toEpochMilli(); | |
ZonedDateTime.ofInstant(Instant.ofEpochMilli(1234567890L), ZoneId.systemDefault()).format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | |
ZonedDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | |
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss").withZone(ZoneOffset.UTC); | |
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss").withZone(ZoneOffset.UTC); | |
import java.time.Instant; | |
import java.time.ZoneId; | |
import java.time.ZoneOffset; | |
import java.time.ZonedDateTime; | |
import java.time.format.DateTimeFormatter; | |
class A { | |
public void foo() { | |
ZonedDateTime.parse("2024-10-25T15:45:00", DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | |
ZonedDateTime.parse("2024-10-25T15:45:00", DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")).toInstant().toEpochMilli(); | |
ZonedDateTime.ofInstant(Instant.ofEpochMilli(1234567890L), ZoneId.systemDefault()).format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | |
ZonedDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss")); | |
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss").withZone(ZoneOffset.UTC); | |
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss").withZone(ZoneOffset.UTC); | |
} |
@@ -65,3 +65,7 @@ dependencies { | |||
tasks.withType(Javadoc::class.java) { | |||
exclude("**/PlanJavaMigration.java") | |||
} | |||
|
|||
tasks.test { | |||
maxHeapSize = "2g" // Set max heap size to 2GB or adjust as necessary |
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.
Surprised we're having to set this here and not generally in for instance
https://github.com/openrewrite/rewrite-build-gradle-plugin/blob/ce39a207adb0bb0c657517f9eb32853b83152e18/src/main/java/org/openrewrite/gradle/RewriteJavaPlugin.java#L102-L131
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.
Yeah we should probably set a higher memory limit than the default 512MB there. I am a bit surprised that this project needs more than that, though. The unit tests shouldn't be particularly demanding on memory, but maybe something has increased our consumption from the last time I looked
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.
I believe applyTemplate operation is very heavy on memory. And this JodaTime has too many apply template calls. Everytime it OOMed on applyTemplate call.
What's changed?
Added migration templates for few more joda-time methods, including methods from the following classes:
Additionally, introduced support for migrating methods implemented by two different classes that do not share the same class hierarchy.
Note: I had to increase heap size for test task. The tests were failing with OutOfMemory error.
Checklist