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.

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

feat: introduce v2 client with new method signatures #543

Merged
merged 4 commits into from
Dec 14, 2022

Conversation

dwsupplee
Copy link
Contributor

This PR includes the changes in #542. Once that is merged I'll update here and the diff should clean up a little bit.

I opted to dupe a bit of the code between the unit test/gapic generators into the "V2" based classes in lieu of engineering out something more DRY for the following reasons:

  • We aren't 100% on whether the new client will be the final approach
  • If this is indeed final approach, we will likely remove the "legacy" generators in short order anyways

Open to discussion on this if anyone feels like that wasn't the right call 👍 .

I'll update the goldens for the integration tests once #542 / #540 are merged.

Please note, don't take the way the new client looks as final (e.g., the base client is still a class and not abstract) - still plenty of room for discussion on how we want that to look. The primary purpose of this PR is to move forward with how the new signatures will look/work.

@dwsupplee dwsupplee requested review from a team as code owners December 5, 2022 20:23
@dwsupplee dwsupplee force-pushed the sig_updates_new_client branch 2 times, most recently from 921ca26 to d6c1536 Compare December 5, 2022 20:52
@dwsupplee dwsupplee force-pushed the sig_updates_new_client branch from d6c1536 to f5b5852 Compare December 5, 2022 21:06
@dwsupplee
Copy link
Contributor Author

dwsupplee commented Dec 5, 2022

The remaining failing tests are due to lack of formatting methods in the v2 gapics, which we will be tagging on with some new work @noahdietz is doing.

Copy link
Collaborator

@noahdietz noahdietz left a comment

Choose a reason for hiding this comment

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

loving it so far!

Copy link
Collaborator

@noahdietz noahdietz left a comment

Choose a reason for hiding this comment

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

:shipit:

@dwsupplee dwsupplee enabled auto-merge (squash) December 14, 2022 22:04
@dwsupplee dwsupplee merged commit da0d00c into v2-dev Dec 14, 2022
@dwsupplee dwsupplee deleted the sig_updates_new_client branch December 14, 2022 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants