-
Notifications
You must be signed in to change notification settings - Fork 90
feat: Add a little bit of typing to google.api_core.retry #453
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
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
@@ -54,13 +54,14 @@ def check_if_exists(): | |||
|
|||
""" | |||
|
|||
from __future__ import unicode_literals | |||
from __future__ import annotations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this library has python_requires >= 3.7
so (1) unicode_literals
does nothing and (2) this is the first version which supports __future__.annotations
it might be a good idea to run https://github.com/asottile/pyupgrade on this codebase to remove the remaining python 2isms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I've captured this TODO in #524
👋 I've rebased this to resolve conflicts (again) -- could it get reviewed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @asottile-sentry!
@@ -54,13 +54,14 @@ def check_if_exists(): | |||
|
|||
""" | |||
|
|||
from __future__ import unicode_literals | |||
from __future__ import annotations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I've captured this TODO in #524
This review does not reference the most recent commit, and you are using the secure version of merge-on-green. Please re-review the most recent commit.
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: