-
Notifications
You must be signed in to change notification settings - Fork 84
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
feat(datestrings): Allow string dates, closes #1765 #1916
Conversation
@ismailsimsek will this do? |
@dark0dave it looks good to me +1 |
Signed-off-by: dark0dave <[email protected]>
Should be ready now. Apologies, its the holiday season. |
@ismailsimsek let me know if you need any other types for dates to be added. Longs/shorts/floats etc. |
@aribray this is ready for review please :), thank you. |
@@ -710,6 +716,12 @@ private static void fillRepeatedField( | |||
} | |||
break; | |||
case STRING: | |||
if (fieldSchema != null && fieldSchema.getType() == TableFieldSchema.Type.DATE) { | |||
if (val instanceof String) { | |||
protoMsg.setField(fieldDescriptor, (String) val); |
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.
Should be repeated field?
TestDate.newBuilder() | ||
.setTestString(18935) | ||
.setTestLong(18935) | ||
.setTestIsoString("1999-01-01") |
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.
Add a test for repeated field.
is there any update on this feature? |
Waiting for the owner to address review feedbacks? If I didn't hear him for a week, I will submit a patched PR. |
Closes #1765
Signed-off-by: dark0dave [email protected]