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

Migration to Java8 #286

Closed
ggalmazor opened this issue Mar 27, 2018 · 2 comments
Closed

Migration to Java8 #286

ggalmazor opened this issue Mar 27, 2018 · 2 comments

Comments

@ggalmazor
Copy link
Contributor

While working on #272, I discovered some weird inconsistencies on how we deal with dates, times and timezones in JR. My rationale for pursuing this was "since we're using Joda Time for date/time stuff, we shouldn't implement anything date/time related and delegate on Joda, which is far more battle-tested than JR".

But then, why don't we migrate JR to Java8 and use the official, native, immutable and generally-nicer new Date and Time API?

By migrating to Java8 we wouldn't only benefit from this new API. Java8 comes with other new APIs and improvements aimed to work better with efficiency focused code (Streams, new concurrency artifacts, etc.) (and Lambdas!). I'm a newcomer to JR but it feels like doing this would open a lot of room for improvements.

The other projects in ODK that depend on JR are already (or will be in the near future) using Java8, so, a priori, it seems like a safe move.

I don't know what's the specific situation with Collect, though.

Let's hear your opinions!

@lognaturel
Copy link
Member

Collect uses Java 8 already (https://github.com/opendatakit/collect/blob/4fbbefe5c0451b0a0592fae4bda59bdc2723cea1/collect_app/build.gradle#L146) so this sounds great to me!

@ggalmazor
Copy link
Contributor Author

ggalmazor commented Apr 4, 2018

OK, I've spent some time exploring the consequences of migrating to Java8. The migration on itself it's pretty easy: just bump source&target version to 1.8 on Gradle and Maven. I'm creating a PR to deal with that.

Regarding the Date/Time API, we can start by removing Joda from the dependencies list and migrate all depending code to the new Date/Time API.

That alone would be great, but then we should migrate all the code that depends on java.util.Date to use the new Date/Time API:

  • The parse/manipulation code is well covered with tests and won't get us in much trouble.
  • The date and time formatting code is not covered by tests and has some custom cases like a home-made l10n engine. Working these will require more effort:
    • Write test harnesses to guarantee that we don't break anything after refactoring.
      • I'm pretty sure that some function implementations like decimal-date-time() are broken for leap days, by the way.
    • Refactor and base everything on the new API's family of classes, which include:
      • LocalTime
      • LocalDate
      • LocalDateTime
      • OffsetDateTime
      • ZonedDateTime
      • Year
      • Month
      • Duration

After merging the PR to upgrade the code to Java8, I can create issues for these things and we can continue discussing how to break this up into manageable pieces.

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

No branches or pull requests

3 participants