Skip to content

Conversation

busunkim96
Copy link
Contributor

The clients were generated with the version name in the name of the clients, so the synth script does a lot of renaming.

@busunkim96 busunkim96 added do not merge Indicates a pull request not ready for merge, due to either quality or timing. api: phishingprotection Issues related to the Phishing Protection API. labels Apr 24, 2019
@busunkim96 busunkim96 requested a review from crwilcox as a code owner April 24, 2019 22:39
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 24, 2019
Copy link
Contributor

@tseaver tseaver left a comment

Choose a reason for hiding this comment

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

Would we be better off fixing the upstream source of the version-mangled file and class names?

@busunkim96
Copy link
Contributor Author

Unfortunately it's not currently possible to rename the client upstream. Ruby also renamed the client using synth.

every service has to have a unique name without considering proto packages. The intention (as I understand it) is to keep this name for the beta, but change it to not include the version number for V1, in the hope that the issue will be fixed before V2

@busunkim96 busunkim96 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 30, 2019
@busunkim96 busunkim96 changed the title WIP: Initial generation of Phishing Protection. Initial generation of Phishing Protection. Apr 30, 2019
unit_test = Path("tests/unit/gapic/v1beta1/test_phishing_protection_service_v1_beta1_client_v1beta1.py")

files = [client, client_config, transport, unit_test]
for f in files:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised flake8 isn't complaining about one-character variable name here. Even if we do change it to f, we need to change how it is used inside the loop, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eek, sorry about that. I don't think synth.py is currently checked by flake or black.

@tseaver
Copy link
Contributor

tseaver commented Apr 30, 2019

Pubsub, Storage, and Logging failures are due to a weird, already-fixed-on-master pubsub issue.

@busunkim96 busunkim96 merged commit 84e24d1 into googleapis:master Apr 30, 2019
parthea pushed a commit that referenced this pull request Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: phishingprotection Issues related to the Phishing Protection API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants