-
Notifications
You must be signed in to change notification settings - Fork 372
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
spec: created_at is a Timestamp. #331
spec: created_at is a Timestamp. #331
Conversation
CC @xing-yang @jingxu97 for FYI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Approve
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any chance we could rename this field to creation_time
or creation_date
while we are changing it? The non-nominal field name kind of bugs me.
I'm happy to make that change later tonight or tomorrow morning... if
people want it
…On Tue, Nov 13, 2018, 6:47 PM Julian Hjortshoj ***@***.***> wrote:
***@***.**** requested changes on this pull request.
any chance we could rename this field to creation_time or creation_date
while we are changing it? The non-nominal field name kind of bugs me.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#331 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACPVLJhb07EUH4CXatMSv1lxC268uezMks5uu1olgaJpZM4YcvOC>
.
|
I'm fine with it too. Go for it. I'll wait for it. |
Thanks @jdef! Yes, Jing and I are fine with the change. Renaming it to creation_time sounds good. |
@julian-hj I've renamed the field - PTAL |
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #330