Change operationIds of SignalRService from Signalr to SignalR #2894
Conversation
Automation for azure-sdk-for-nodeNothing to generate for azure-sdk-for-node |
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-libraries-for-javaThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-goThe initial PR has been merged into your service PR: |
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback Thanks for your co-operation. |
| ], | ||
| "description": "Lists all of the available REST API operations of the Microsoft.SignalRService provider.", | ||
| "operationId": "Signalr_ListOperations", | ||
| "operationId": "SignalR_ListOperations", |
There was a problem hiding this comment.
You should call this one Operations_List. It will be consistent with everybody else:
| ], | ||
| "description": "Checks that the SignalR name is valid and is not already in use.", | ||
| "operationId": "Signalr_CheckNameAvailability", | ||
| "operationId": "SignalR_CheckNameAvailability", |
There was a problem hiding this comment.
Why don't you call the operation ID directly "Accounts"? right now, this is redundant: we know it's signal R and gives Python code like that (but I guess it will be close in other language as well):
import azure.mgmt.signalr
client = azure.mgmt.signalr.SignalRManagementClient(**parameters)
client.signal_r.create_or_update(**parameters)wouldn't be better with:
import azure.mgmt.signalr
client = azure.mgmt.signalr.SignalRManagementClient(**parameters)
client.accounts.create_or_update(**parameters)There was a problem hiding this comment.
Note that I said "accounts", but I mean "something with is not the 'signal_r' string"
There was a problem hiding this comment.
We decided to using Signalr for OperationId/Schema Id for the client SDKs.
There was a problem hiding this comment.
See PR #2904. In the new version, it would look like:
import azure.mgmt.signalr
client = azure.mgmt.signalr.SignalrManagementClient(**parameters)
client.signalr.create_or_update(**parameters)
To keep the name consistent, we updated all Schema Ids such as SignalrResource, SignalrManagementClient. I checked the generated Python/.Net SDK files, the file names and class names are expected as Signalr. SignalR will only exist in comments, descriptions and RP namespace.
sergey-shandar
left a comment
sergey-shandar
left a comment
There was a problem hiding this comment.
@juniwang please, address issues in another PR. Let me know the number of the new PR so I can assign it to myself.
|
Hi There, I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result: File: AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback Thanks for your co-operation. |
|
@lmazuel could you approve this PR? I can't merge without your approval. |
6 participants
We used
Signalrbecause the python SDK class names will look better(signalr_XXXinstead ofsignal_rXXX). But I confirmed with Python SDK team the names are transparent to customers hence that won’t be an issue. In this PR, we are going roll back the originalSignalRTested on local follow https://github.com/Azure/adx-documentation-pr/wiki/Azure-Swagger-Tools. All validations passed.