-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
datastore: support civil.Date for properties #3089
Comments
This seems like a good thing to add support for. @IlyaFaer could you take this one? The |
@tritone, sure, I'll take a look. |
We've merged this change. @benjaminjkraft would it be possible for you to test this for your purposes at HEAD before we cut a release? |
Sure! Let me follow up with @MiguelCastillo and/or @dnerdy but I think we should be able to pull this in and get some testing soon. Thanks much for implementing this, it will definitely simplify some things for us! |
I've opened a separate issue for the bug: #3360. |
Is your feature request related to a problem? Please describe.
We sometimes store dates-without-location (in the sense of the
civil
package) in datastore. Sadly, that means we have to handle them, temporarily at least, astime.Time
. This is sort of a foot-gun for all the usual reasons (described, for example, in e.g. golang/go#19700).Describe the solution you'd like
We could have a struct-field of type
civil.Date
; that way we never have to have atime.Time
-representing-a-civil-date floating around. Ideally this would be compatible with how the libraries in other languages, some of which have a date-property, do things.Describe alternatives you've considered
We could write a PropertyLoadSaver for each such model, and specially handle the field. That's definitely possible but it's some extra work for each such model.
The text was updated successfully, but these errors were encountered: