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鈥檒l 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

@jimfulton jimfulton commented Jul 23, 2021

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 Jul 1, 2021
It, itself isn't public.
and updated package name in copyright statement.
Co-authored-by: Tim Swast <swast@google.com>
tswast
tswast approved these changes Aug 6, 2021
Copy link
Collaborator

@tswast tswast left a comment

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

@jimfulton jimfulton commented Aug 6, 2021

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 as a code owner Aug 6, 2021
@jimfulton jimfulton requested a review from leahecole Aug 6, 2021
@snippet-bot
Copy link

@snippet-bot 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

@jimfulton jimfulton commented Aug 6, 2021

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

@jimfulton jimfulton commented Aug 6, 2021

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

@jimfulton jimfulton commented Aug 10, 2021

The diff is clean after merging master.

@jimfulton
Copy link
Contributor Author

@jimfulton jimfulton commented Aug 11, 2021

@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.

tswast
tswast approved these changes Aug 11, 2021
Copy link
Collaborator

@tswast tswast left a comment

Looks good!

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

@jimfulton jimfulton merged commit da7a403 into googleapis:master Aug 17, 2021
9 checks passed
@jimfulton jimfulton deleted the geography2 branch Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants