Fix missing UTC timezone "Z" in Sqlite UTCTime strings#1585
Fix missing UTC timezone "Z" in Sqlite UTCTime strings#1585parsonsmatt merged 4 commits intoyesodweb:masterfrom
Conversation
malteneuss
force-pushed
the
fix_sqlite_utctime_text
branch
from
26f3a8a to
97e1bb7
Compare
malteneuss
changed the title
malteneuss
changed the title
malteneuss
force-pushed
the
fix_sqlite_utctime_text
branch
from
97e1bb7 to
2d08c82
Compare
malteneuss
changed the title
malteneuss
force-pushed
the
fix_sqlite_utctime_text
branch
2 times, most recently
from
80a4a74 to
7ac704f
Compare
|
I can't format PersistField.hs due to (using fourmolu from $ fourmolu -i persistent/Database/Persist/Class/PersistField.hs
persistent/Database/Persist/Class/PersistField.hs:345:5-20
The GHC parser (in Haddock mode) failed:
[GHC-58481] parse error on input `fromPersistValue'Maybe someone can tell me what's wrong from the logs? I can't read what restyled wrote due to permissions... |
malteneuss
force-pushed
the
fix_sqlite_utctime_text
branch
2 times, most recently
from
16d110a to
b4430b0
Compare
parsonsmatt
left a comment
parsonsmatt
left a comment
There was a problem hiding this comment.
Code looks great, thanks for putting this together!
Don't worry about the restyled. That's complaining about the CPP for some reason.
I have a question/concern on backwards/forwards compatibility with persistent-sqlite and the new formatting. We may be breaking downstream users if the timestamp formatting changes in versions, so we may need to be a little more careful about how we do this.
|
If you want to fix the issue with restyled, here's the PR that does it: https://github.com/yesodweb/persistent/pull/1586/files We'll conflict if I merge first. I'm happy to merge this one in first and fix the conflict later, but each CPP block needs to have a full declaration in it, so that's why it's breaking. |
|
Since this restyled action touches a more than my changes, i'd like to post pone it. |
|
I will squash and rebase before the final merge |
Co-authored-by: Matt Parsons <[email protected]>
malteneuss
force-pushed
the
fix_sqlite_utctime_text
branch
from
44fc521 to
3006c07
Compare
|
Squashed |
|
fyi: I don't have merge rights. Anything else i can do? E.g. there seems to be an issue with the GHC 8.8 pipeline, but i can't read the logs. Or is there maybe a waiting period? |
|
Logs: |
|
@parsonsmatt should be good now |
|
Great, thanks! I'll try to get this released today |
2 participants
I noticed that the Sqlite formatting from UTCTime values to string (sqlite doesn't support datetime natively) is missing the UTC timezone marker "Z". So it should be e.g. "2025-04-12T06:53:42Z" instead of "2025-04-12T06:53:42".
I made this change is non-breaking (UTC timezone was implicitly assumed before) by keeping the old parsing logic so new and old format can be read from existing databases.
Loosely related to #1542 #1117 and #339
Before submitting your PR, check that you've:
@sincedeclarations to the Haddockfourmoluon any changed files (restyledwill do this for you, soaccept the suggested changes if it makes them)
.editorconfigandfourmolu.yamlfiles for details)After submitting your PR:
(unreleased)on the Changelog