[WIP] Removes JodaTime #157
Conversation
non-standard code, fixes test compilation
|
Thanks for the effort! I guess you saw conversation in #131 |
|
@oshai No worries! Yep, I saw the discussion. In the event that I get this working I'll bump the version to 2.0.0 since this would be a breaking change. Will add some notes to the README as well in that case RE: how to migrate from 1.x to 2.x. |
| @@ -1,18 +1,10 @@ | |||
| package com.github.jasync.sql.db.column | |||
|
|
|||
| import sun.net.util.IPAddressUtil.textToNumericFormatV4 | |||
There was a problem hiding this comment.
This was not compiling for me using JDK 11 (AdoptOpenJDK, windows), and generally using sun.* packages is discouraged - decided to replace with standard java library calls. Let me know if there's a reason this textToNumeric() call was made in the first place - seems like .getByName() works OK
There was a problem hiding this comment.
I think I checked it and this method not exists in java 8
There was a problem hiding this comment.
Yep, the textToNumeric methods aren't public/don't exist or something. That's why I decided to remove them.
|
hey @oshai I made some progress and fixed all of the common tests. However, I'm running into a strange exception with the MySQL tests. It seems one test is failing due to: Does this sound familiar to you? I haven't been able to figure out what's causing it. Thanks |
|
Try to run the test seperatly and provide full stacktrace. Right now it doesn't looks familiar. |
|
@bakatz - whats the status of this PR? are you still planning on working on it? |
|
@oshai not exactly - haven't had time to work on this due to work commitments - feel free to take the PR over if you have time |
|
@bakatz I don't have the time to do that as well, just thinking how we can continue that effort. |
|
Stale pull request message |
I don't want a dependency on JodaTime for my projects, so I decided to start fixing #131
TODO:
[x] Common library
[x] MySQL
[x] Fix TimestampEncoder
[ ] Fix MySQL tests
[] Postgres
[] Fix Postgres tests
Looking for comments on this before I finish the whole thing (ideally from someone who has in depth knowledge of
java.time) - I'm not 100% sure I've converted the calls successfully in some cases. Mostly followed this as a guide: https://blog.joda.org/2014/11/converting-from-joda-time-to-javatime.htmlImplementation notes:
TimestampEncoder is definitely broken, probably need to use ZonedDateTime instead of LocalDateTime etc. Will take a look at it later and resolve.Even though TimestampEncoder assumes no time zone, it formats times using a 'Z' suffix to represent the UTC timezone. As such, I had to make it useZonedDateTimeotherwise a formatting exception would occur at runtime.Cheers