GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Switch to JSR 310 Date and Time API by dragneelfps · Pull Request #233 · jasync-sql/jasync-sql Conversation
|
|
Getting following in CI build.. :/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hi, thanks for the PR! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I could try to have Joda Time supported as well. But the thing is, do we really want to continue supporting it? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Bump version to 2.0.0 sounds reasonable to me. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
a6c447a to
6ea3c8f
Compare
|
Note that some tests are still failing on CI. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I fixed most. Just having issues understanding logic behind jasync-sql/postgresql-async/src/test/java/com/github/aysnc/sql/db/integration/TimeAndDateSpec.kt Line 151 in 152cdef
can you help, @oshai ? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
I didn't wrote it originally, but from reading the code I think it checks the following:
There might be a problem either with the timezone or the millis if I have to guess. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I noticed something, postgres is returning timestamps in UTC string, even when I insert it with non UTC offset. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Codecov Report
@@ Coverage Diff @@ ## master #233 +/- ## ============================================ + Coverage 80.03% 80.17% +0.14% - Complexity 1074 1079 +5 ============================================ Files 265 266 +1 Lines 3916 3919 +3 Branches 522 523 +1 ============================================ + Hits 3134 3142 +8 + Misses 534 531 -3 + Partials 248 246 -2
Continue to review full report at Codecov.
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
- Removes joda from encoding/decoding - Using Threeten for PeriodDuration
According to https://www.postgresql.org/docs/9.1/datatype-datetime.html 8.5.2. Date/Time Output: Conversion on test makes sense (to see it's the same date as inserted). It's just not clear how this was changed from pervious impl. I still need to review all changes, but it will take some time. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the PR.
I left some comments in code and have some more general things:
About the migration to 2.x: I am thinking if we can add a suggestion to users how to do that more easily? Do you have any suggestion?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| created_at_date DATE not null, | ||
| created_at_datetime DATETIME not null, | ||
| created_at_timestamp TIMESTAMP not null, | ||
| created_at_datetime DATETIME(6) not null, |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
why was it changed?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Can you point me to which cases are you talking about specifically?
Yes. Java uses nano, joda uses millis, db micro(at least in mysql) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Look at LocalDateTimeEncoderDecoder:17 and see more examples in 'files changed' with the warning: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@dragneelfps before merging there was still one answered question in the review about In addition, will you have some time to improve test coverage? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Stale pull request message |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I will merge this PR soon. Will create 1.x branch before that. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for the PR! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.