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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve issue importing library #64

merged 4 commits into from Aug 20, 2021


Copy link

@parthea parthea commented Aug 16, 2021

Fixes #63 馃

@parthea parthea requested a review from as a code owner Aug 16, 2021
@product-auto-label product-auto-label bot added the api: gkehub label Aug 16, 2021
@google-cla google-cla bot added the cla: yes label Aug 16, 2021
@parthea parthea requested a review from busunkim96 Aug 16, 2021
docs/gkehub_v1/configmanagement_v1/types.rst Show resolved Hide resolved
google/cloud/gkehub_v1/types/ Outdated Show resolved Hide resolved Outdated Show resolved Hide resolved Show resolved Hide resolved Show resolved Hide resolved
Copy link

@busunkim96 busunkim96 commented Aug 16, 2021

configmanagement and multiclusteringress to live under the gogole/cloud/gkehub/v1 in the protos so this seems like the right thing to do.

Copy link

@busunkim96 busunkim96 left a comment

configmanagement and multiclusteringress live under the google/cloud/gkehub/v1 in the protos so this new namespacing seems correct. Leaving the types here also SGTM, since the protos are clearly nested under a specific version of gkehub.

However, I believe we agreed on generating library types through proto-plus (like python-access-approval). In the event either of these sub packages turns into its own GAPIC (which does look pretty unlikely in this case) the change will be non-breaking. IMO the library types being consistently proto-plus also provides a better end user experience.

For more discussion on a similar issue, see googleapis/python-access-context-manager#42. Show resolved Hide resolved
@parthea parthea force-pushed the fix-dependency-import branch from 4b29b3d to 9f3cd73 Compare Aug 20, 2021
Copy link
Contributor Author

@parthea parthea commented Aug 20, 2021

@busunkim96 Please could you review/approve?

@parthea parthea added the automerge label Aug 20, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit cfa166e into master Aug 20, 2021
10 checks passed
@gcf-merge-on-green gcf-merge-on-green bot deleted the fix-dependency-import branch Aug 20, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge label Aug 20, 2021
gcf-merge-on-green bot pushed a commit that referenced this issue Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
api: gkehub cla: yes
None yet

Successfully merging this pull request may close these issues.

3 participants