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

Moving Bigtable helpers for duration protobufs into core. #2952

Merged
merged 1 commit into from
Jan 20, 2017

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jan 20, 2017

Note this is from a private branch of @tseaver. Though it largely copies a function from Bigtable so I'm not sure how to proceed here.

@tseaver Can you weigh in?

/cc @geigerj

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 20, 2017
@tseaver
Copy link
Contributor

tseaver commented Jan 20, 2017

@dhermes It looks like the two differ for negative durations, which means the version in Bigtable is more likely correct. Why don't we promote that function (and the duration_pb_to_timedelta companion) up from Bigtable to google.cloud._helpers instead?

@dhermes dhermes force-pushed the upstream-timestamp-changes branch from ba4c2e1 to dd25e45 Compare January 20, 2017 19:10
@dhermes
Copy link
Contributor Author

dhermes commented Jan 20, 2017

@tseaver SGTM. I wanted to double-check before doing that. PTAL.

I just went with the existing name: _timedelta_to_duration_pb (vs. _timedelta_to_pb_duration in your impl.). WDYT?

@dhermes dhermes changed the title Adding support for converting a timedelta to a duration. Moving Bigtable helpers for duration protobufs into core. Jan 20, 2017
@dhermes dhermes added the api: bigtable Issues related to the Bigtable API. label Jan 20, 2017
@tseaver
Copy link
Contributor

tseaver commented Jan 20, 2017

I just went with the existing name: _timedelta_to_duration_pb (vs. _timedelta_to_pb_duration in your impl.). WDYT?

Works for me: I will adjust the private branch accordingly after merging from master.

@dhermes dhermes merged commit 172d788 into googleapis:master Jan 20, 2017
@dhermes dhermes deleted the upstream-timestamp-changes branch January 20, 2017 19:38
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
…anges

Moving Bigtable helpers for duration protobufs into core.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the Bigtable API. api: core cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants