Skip to content

fix: Add type hints for iam api-client samples #9977

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

Merged
merged 3 commits into from
May 18, 2023

Conversation

glasnt
Copy link
Contributor

@glasnt glasnt commented May 17, 2023

Description

Fixes b/280879750

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: iam Issues related to the Identity and Access Management API. labels May 17, 2023
@glasnt glasnt marked this pull request as ready for review May 17, 2023 03:24
@glasnt glasnt requested a review from a team as a code owner May 17, 2023 03:24
Copy link
Member

@rsamborski rsamborski left a comment

Choose a reason for hiding this comment

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

Thanks, approved.
Future ask: please remember to add a link to the bug in the PR description for reference (I've added it myself this time).

@kweinmeister
Copy link
Contributor

LGTM. Just curious, there are several # type: ignore comments added. These are impractical to address?

@glasnt
Copy link
Contributor Author

glasnt commented May 17, 2023

The # type: ignore were to convince mypy not to inspect the types of the imports. Fixit guidance said to use mypy, but CI uses flake8-annotations instead. I'm happy to remove these.

@rsamborski
Copy link
Member

rsamborski commented May 18, 2023

@kweinmeister wrote:

LGTM. Just curious, there are several # type: ignore comments added. These are impractical to address?

@glasnt followed official guidance (see here), so I think we should leave as is.

@rsamborski rsamborski added the automerge Merge the pull request once unit tests and other checks pass. label May 18, 2023
@gcf-merge-on-green gcf-merge-on-green bot merged commit f39e919 into main May 18, 2023
@gcf-merge-on-green gcf-merge-on-green bot deleted the fixit-glasnt-typehints-iam-api-client branch May 18, 2023 09:30
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: iam Issues related to the Identity and Access Management API. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants