TypeSpec migration post items from reviewers #3190
Description
Buddy review comments
| Comment | Link examples (I don't need to add all the links related to the same category) | Action |
|---|---|---|
| Use canonical name for resource name (and other related stuff) | Azure/azure-rest-api-specs#34281 (comment) Azure/azure-rest-api-specs#34281 (comment) Azure/azure-rest-api-specs#34281 (comment) Azure/azure-rest-api-specs#34318 (comment) | New linter: Update ResourceNameParameter template and linting rules to encourage minimal parameter usage · Issue #3017 · Azure/typespec-azure Converter fix: Align resource model name to type name · Issue #5141 · Azure/autorest |
| Optional/Empty body related (and request body differs from response body) | Azure/azure-rest-api-specs#34281 (comment) Azure/azure-rest-api-specs#35243 (comment) | If we take the comment, it means break API. We tend to keep the existing API shape. To don't break API, we've added the support in legacy templates. |
| Standard list response | Azure/azure-rest-api-specs#34281 (comment) Azure/azure-rest-api-specs#35243 (comment) | Issue: Customize response model name for list operation · Issue #3049 · Azure/typespec-azure |
| Client location issue | Azure/azure-rest-api-specs#34278 (comment) | Handled by [TCGC&ARM] introduce a new decorator to provide backward compatible to swagger @operationId · Issue #2683 · Azure/typespec-azure |
| Missing doc | Azure/azure-rest-api-specs#34278 (comment) | Adding missing doc will introduce a lot of diffs which causes huge effort for us to do the comparison, we tend to keep it as empty. |
| Remove private decorators | Azure/azure-rest-api-specs#34278 (comment) Azure/azure-rest-api-specs#34278 (comment) | Already removed. (some PRs inherited from wave 1 might exist, will check) |
| Remove linter suppression | Issue: Solutions to remove linter suppressions · Issue #2831 · Azure/typespec-azure | |
| PrivateLinkResourceListResult issue | Azure/azure-rest-api-specs#34278 (comment) | Fixed. |
| Suppression justification | Azure/azure-rest-api-specs#34069 (comment) | We've followed what Laurent suggested and already populated the short link |
| Extension resource | Azure/azure-rest-api-specs#34496 (review) | Mark released the extension resource feature and we've applies to this PR. |
| Don't use ArmResourceDeleteWithoutOkAsync | Azure/azure-rest-api-specs#35243 (comment) | Issue: #3039 |
| Don't use legacy operation to rename a parameter | Azure/azure-rest-api-specs#35243 (comment) Azure/azure-rest-api-specs#35243 (comment) | Issue: #3016 |
| File naming | Azure/azure-rest-api-specs#34326 (comment) | Already discussed in the broader team. Issue: Unify file naming · Issue #5140 · Azure/autorest |
| Some other comments | Azure/azure-rest-api-specs#34324 (comment) Azure/azure-rest-api-specs#34278 (comment) Azure/azure-rest-api-specs#34278 (comment) Azure/azure-rest-api-specs#34069 (comment) Azure/azure-rest-api-specs#34069 (comment) Azure/azure-rest-api-specs#34069 (comment) | Replied or explained. |
ARM review comments
| Comment category | Link examples (I don't need to add all the links related to the same category) | Service Impact | Action |
|---|---|---|---|
| Use common type instead of defined self | Azure/azure-rest-api-specs#34318 (comment) Azure/azure-rest-api-specs#34318 (comment) Azure/azure-rest-api-specs#34336 (comment) | Deviceprovisioningservices, guestconfiguration | Script to check any model similar to common type, ask service team to improve |
| Private decorators | Azure/azure-rest-api-specs#34318 (comment) | deviceprovisioningservices | Remove private decorates later. |
| Privateendpointconnection related | Azure/azure-rest-api-specs#34318 (comment) Azure/azure-rest-api-specs#34078 (comment) Azure/azure-rest-api-specs#34323 (comment) | Deviceprovisioningservices, elasticsan, recoveryservices | We have new design for privateendpointconnection, should check existing spec to see if need to change to the new design. |
| OperationId related | Azure/azure-rest-api-specs#34238 (comment) | edgeorder | Will check existing spec later |
| String -> url | Azure/azure-rest-api-specs#34238 (comment) Azure/azure-rest-api-specs#34238 (comment) Azure/azure-rest-api-specs#34336 (comment) | Edgeorder, guestconfiguration | Will have a script that and property ending with url but has type string. |
| Unrealistic example | Azure/azure-rest-api-specs#34078 (comment) Azure/azure-rest-api-specs#34078 (comment) | elasticsan | Need to clarify |
| Custom legacy operations | Azure/azure-rest-api-specs#34336 (comment) | guestconfiguration | Remove the custom legacy operations |
| Id -> resource id/uuid | Azure/azure-rest-api-specs#34336 (comment) https://github.com/Azure/azure-rest-api-specs/pull/34336/files/5c4ad69b0109e2e2a4b5f0be01d975d1301147b8#r2168036614 Azure/azure-rest-api-specs#34323 (comment) | Guestconfiguration, recoveryservices | Will have a script that and property ending with id but has type string. |
| Model named resource/trackedresource/proxyresource | https://github.com/Azure/azure-rest-api-specs/pull/34336/files/5c4ad69b0109e2e2a4b5f0be01d975d1301147b8#r2168034791 https://github.com/Azure/azure-rest-api-specs/pull/34336/files/5c4ad69b0109e2e2a4b5f0be01d975d1301147b8#r2168037967 | guestconfiguration | We are not expected to have a model called Resource |