FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Moving Bigtable helpers for duration protobufs into core. by dhermes · Pull Request #2952 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

Moving Bigtable helpers for duration protobufs into core. - #2952

Merged
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:upstream-timestamp-changes
Jan 20, 2017
ConversationCommits (1)ChecksFiles changed
Merged

Moving Bigtable helpers for duration protobufs into core.#2952
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:upstream-timestamp-changes

Conversation

dhermes commented Jan 20, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 added the cla: yes This human has signed the Contributor License Agreement. label Jan 20, 2017

tseaver commented Jan 20, 2017

Copy link
Copy Markdown
Contributor

@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 force-pushed the upstream-timestamp-changes branch from ba4c2e1 to dd25e45 Compare January 20, 2017 19:10

dhermes commented Jan 20, 2017

Copy link
Copy Markdown
Contributor Author

@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 changed the title Adding support for converting a timedelta to a duration. Moving Bigtable helpers for duration protobufs into core. Jan 20, 2017
dhermes added the api: bigtable Issues related to the Bigtable API. label Jan 20, 2017

tseaver commented Jan 20, 2017

Copy link
Copy Markdown
Contributor

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 merged commit 172d788 into googleapis:master Jan 20, 2017
dhermes deleted the upstream-timestamp-changes branch January 20, 2017 19:38
dhermes mentioned this pull request Jan 30, 2017
Merged
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
Merge pull request googleapis#2952 from dhermes/upstream-timestamp-ch…
c9e6ef9 
…anges

Moving Bigtable helpers for duration protobufs into core.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
tseaver tseaver approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

tseaver

Labels

api: bigtable Issues related to the Bigtable API. api: core cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL