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.

Sign up for GitHub

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

Jump to bottom

typing changes block synth #188

Closed
tseaver opened this issue Sep 16, 2020 · 5 comments
Closed

typing changes block synth #188

tseaver opened this issue Sep 16, 2020 · 5 comments
Assignees
@tseaver
Labels
api: firestore Issues related to the googleapis/python-firestore API. type: process A process-related concern. May include testing, release, or the like.

Comments

@tseaver
Copy link
Contributor

tseaver commented Sep 16, 2020

PR #134 made manual changes to generated code in order to get pytype running / passing under CI. Until we fix the generator / templates / bazel stuff, we cannot do generation without breaking pytype (synthtool would even remove it from noxfile.py).
@tseaver tseaver added the type: process A process-related concern. May include testing, release, or the like. label Sep 16, 2020
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/python-firestore API. label Sep 16, 2020
This was referenced Sep 16, 2020
Merged
Merged
Closed
Closed
Closed
@tseaver
Copy link
Contributor Author

tseaver commented Sep 16, 2020

I've tagged PRs #170, #171, and #172 as blocked on this issue. Resolving this issue will necessitate doing a manual synth, at which point those PRs should all be closed.

I've tagged PRs #176 and #182 as blocked on this issue, because we should work out how to have pytypes checked under CI without blocking synth, before we add more (potentially breaking) typing-related changes.

@crwilcox
Copy link
Contributor

I have excluded pytype from CI in a recent PR. https://github.com/googleapis/python-firestore/pull/187/files#diff-ce0d2ea7b6085e6c4bd286d0a15a3971R47

@tseaver
Copy link
Contributor Author

tseaver commented Sep 21, 2020

@crwilcox PR #187 actually removed the manual changes applied to generated code to make pytype happy, which does unblock further synth. We still shouldn't move forward on other pytype-related changes (PR #176, PR #182), until we figure out how to get those changes re-applied, ideally via gapic-generator-python, or via synth.py (as a workaround).

@crwilcox
Copy link
Contributor

@tseaver is this still active?

@tseaver tseaver self-assigned this Dec 15, 2020
tseaver added a commit that referenced this issue Dec 15, 2020
@tseaver
chore: manual synth
d69226d 
Closes #188.
@tseaver tseaver mentioned this issue Dec 15, 2020
Closed
@tseaver
Copy link
Contributor Author

tseaver commented Dec 15, 2020

@crwilcox #201 basically fixed it. I've done another manual synth in #279 to ensure that it remains OK.

@tseaver tseaver closed this as completed Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tseaver tseaver

Labels
api: firestore Issues related to the googleapis/python-firestore API. type: process A process-related concern. May include testing, release, or the like.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: manual synth googleapis/python-firestore
2 participants
@tseaver @crwilcox