Navigation Menu

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

feat: Add geography support #228

Merged
merged 130 commits into from Aug 17, 2021
Merged

feat: Add geography support #228

merged 130 commits into from Aug 17, 2021

Conversation

jimfulton
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #136 🦕

jimfulton and others added 30 commits July 1, 2021 11:30
It, itself isn't public.
and updated package name in copyright statement.
Co-authored-by: Tim Swast <swast@google.com>
@jimfulton jimfulton added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 6, 2021
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 6, 2021
Copy link
Collaborator

@tswast tswast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

I wonder if we want to merge this before the rename just to make the docfx session happy?

@jimfulton
Copy link
Contributor Author

I wonder if we want to merge this before the rename just to make the docfx session happy?

I'm working on a branch that combines the rename and geography (and snippets to be used in the geography doc).

I suggest we decide next week whether to wait for Dan to fix docfx or to include geography in alpha 1.

@jimfulton jimfulton requested a review from a team as a code owner August 6, 2021 20:50
@snippet-bot
Copy link

snippet-bot bot commented Aug 6, 2021

Here is the summary of changes.

You are about to add 8 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@jimfulton
Copy link
Contributor Author

I merged the rename branch into this. Maybe that was a mistake.

We'll either want to merge the rename branch first, at which point this will be smaller, or we'll merge this.

@jimfulton
Copy link
Contributor Author

Everything has been reviewed here except: 6343bee, which changes the geography documentation to use snippets, which found a number of bugs. :) I also improved one of the query examples.

I tried to format the query examples to be more readable in the documentation, but black wouldn't let me. :(

@jimfulton
Copy link
Contributor Author

The diff is clean after merging master.

@jimfulton
Copy link
Contributor Author

@tswast I'm going to assume you still approve unless I hear otherwise. 😃

I'm thinking of waiting to merge this until after 1.0.0-a1 is released, unless you want me to include these changes in a1.

Copy link
Collaborator

@tswast tswast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I'm okay with waiting to merge this until after the a1 release.

@jimfulton jimfulton removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 17, 2021
@jimfulton jimfulton merged commit da7a403 into googleapis:master Aug 17, 2021
@jimfulton jimfulton deleted the geography2 branch August 17, 2021 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery-sqlalchemy API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the BigQuery geography type.
3 participants