Skip to content

Conversation

gkevinzheng
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 #<issue_number_goes_here> 🦕

@gkevinzheng gkevinzheng requested review from a team as code owners August 8, 2024 17:09
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Aug 8, 2024
@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/python-logging API. label Aug 8, 2024
owlbot.py Outdated

place_before(
"noxfile.py",
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

I see firestore adds an escape character here. Maybe that's why it's not working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found out that using s.replace instead of place_before makes it work. The escape characters were used because s.replace takes in a regex.

"""Verify type hints are mypy compatible."""
session.install("-e", ".")
session.install("mypy", "types-setuptools")
# TODO: also verify types on tests and logging_v2, big undertakings
Copy link
Contributor

Choose a reason for hiding this comment

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

are those big undertakings? logging_v2 should already have valid types in place. And I'd say tests are optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess big is an overexaggeration, but when I ran mypy there were 22 errors in 12 files.

@daniel-sanche
Copy link
Contributor

you should also add a github action to execute mypy as a presubmit check

@product-auto-label product-auto-label bot added the stale: old Pull request is old and needs attention. label Sep 8, 2024
@product-auto-label product-auto-label bot added stale: extraold Pull request is critically old and needs prioritization. and removed stale: old Pull request is old and needs attention. labels Oct 8, 2024
@parthea parthea assigned gkevinzheng and unassigned daniel-sanche May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the googleapis/python-logging API. size: s Pull request size is small. stale: extraold Pull request is critically old and needs prioritization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants