feat: Remove need to pass location parameter along#1093
feat: Remove need to pass location parameter along#1093danieljbruce merged 12 commits intogoogleapis:mainfrom
Conversation
src/cluster.ts
Outdated
| */ | ||
| setMetadata( | ||
| metadata: SetClusterMetadataOptions, | ||
| metadata: BasicClusterConfig, |
There was a problem hiding this comment.
This is not a breaking change because BasicClusterConfig includes all the parameters that SetClusterMetadataOptions includes. We need to do this to allow the user to pass in location parameter, which shouldn't be required, but seems to be accepted in other codebases when we update clusters.
There was a problem hiding this comment.
I believe this will be a compilation error for upstream users because location is require3d on BasicClusterConfig, what I might do instead is add location as an optional parameter on SetClusterMetadataOptions.
There was a problem hiding this comment.
Great point. By adding this optional parameter to the interface it also means we make fewer changes too which is good because it keeps this PR small. Done.
src/cluster.ts
Outdated
| */ | ||
| setMetadata( | ||
| metadata: SetClusterMetadataOptions, | ||
| metadata: BasicClusterConfig, |
There was a problem hiding this comment.
I believe this will be a compilation error for upstream users because location is require3d on BasicClusterConfig, what I might do instead is add location as an optional parameter on SetClusterMetadataOptions.
…into autoscaler-refactor-tech-debt
danieljbruce
left a comment
There was a problem hiding this comment.
Comments addressed.
src/cluster.ts
Outdated
| */ | ||
| setMetadata( | ||
| metadata: SetClusterMetadataOptions, | ||
| metadata: BasicClusterConfig, |
There was a problem hiding this comment.
Great point. By adding this optional parameter to the interface it also means we make fewer changes too which is good because it keeps this PR small. Done.
This PR is the 2nd step towards refactoring code pertaining to autoscaler and cluster update work. This should be merged after the following PR:
#1092