Skip to content
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

Change the way SpannerDate returns value. #529

Closed
wants to merge 3 commits into from
Closed

Change the way SpannerDate returns value. #529

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 15, 2019

Fixes #526 (it's a good idea to open an issue first for discussion)

  • Tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 15, 2019
Modified Test to match the new logic.
@callmehiphop
Copy link
Contributor

@sulaysumaria so interestingly enough this change will cause an off-by-one error since the Date constructor interprets Y-m-d timestamps UTC-0 and those date methods all return local time values. I think the underlying issue here is that the Date object might not be the wrong way to go since it infers a timezone based on the format of the date string. I'm going to hold off on this PR for now and try to open a dialogue on how to move forward with this bug.

@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants