feat: rename unary LRO for diregapic #956
Conversation
google-cla
bot
added
the
cla: yes
| @@ -24,11 +24,11 @@ | |||
| "retry_codes_name": "non_idempotent", | |||
| "retry_params_name": "default" | |||
| }, | |||
| "Delete": { | |||
| "UnaryDelete": { | |||
There was a problem hiding this comment.
I believe we must keep the original method names in this config because this is only a client change and gax will keep using original names.
| @@ -206,7 +206,7 @@ export class AddressesClient { | |||
| // Iterate over each of the methods that the service provides | |||
| // and create an API call method for each. | |||
| const addressesStubMethods = | |||
| ['aggregatedList', 'delete', 'insert', 'list']; | |||
| ['aggregatedList', 'unaryDelete', 'unaryInsert', 'list']; | |||
There was a problem hiding this comment.
We must not rename the names in the stubs. The stub still has original names taken from the protos.
| @@ -369,23 +369,23 @@ export class AddressesClient { | |||
| 'project': request.project || '', | |||
| }); | |||
| this.initialize(); | |||
| return this.innerApiCalls.delete(request, options, callback); | |||
| return this.innerApiCalls.unaryDelete(request, options, callback); | |||
There was a problem hiding this comment.
same here, let's keep innerApiCalls name just delete. We only want to rename the user-visible method, without touching any internals.
| @@ -438,6 +458,11 @@ function augmentMethod( | |||
| ); | |||
| } | |||
| } | |||
| // Rename the unary LRO method | |||
| if (isUnaryLRO(parameters.service, method, parameters.diregapic)) { | |||
| method.name = 'Unary' + method.name; | |||
There was a problem hiding this comment.
This is not the right approach. The generator code must not know anything about the TypeScript specific. The renames must be done in the templates, there is a Namer functionality there to avoid naming conflicts.
See how we build the names for paginated Async function as an example.
|
Thanks for doing this! Two notes:
|
|
Another approach on #967 |
Rename DIREGAPIC LRO for forward compatibility at GA