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

fix: remove LRO mixin if there are no LRO methods #1262

Merged
merged 1 commit into from
Sep 19, 2022
Merged

Conversation

alexander-fenster
Copy link
Contributor

@alexander-fenster alexander-fenster commented Sep 16, 2022

Do not include LRO mixin methods if an API service yaml enables LRO mixin, but the API does not actually have any LRO methods.

The main code change is in proto.ts, others are just the baseline changes.

Note that this change might be breaking for APIs that have more than one client but only one of those clients have LRO methods. The current implementation will add GetOperation and such to all clients. The changed Showcase baselines show that. I will send a separate PR to have a break glass generator option overriding the list of mixins that I will later use to avoid the possible breaking changes to such APIs.

@alexander-fenster alexander-fenster merged commit 2d28ae4 into main Sep 19, 2022
@alexander-fenster alexander-fenster deleted the unused-lro branch September 19, 2022 21:48
gcf-merge-on-green bot pushed a commit that referenced this pull request Sep 20, 2022
🤖 I have created a release *beep* *boop*
---


## [2.18.0](v2.17.0...v2.18.0) (2022-09-20)


### Features

* An option to override mixins ([#1266](#1266)) ([2546855](2546855))


### Bug Fixes

* Remove LRO mixin if there are no LRO methods ([#1262](#1262)) ([2d28ae4](2d28ae4))
* Use fully qualified request type name in tests ([#1267](#1267)) ([238156e](238156e))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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