-
Notifications
You must be signed in to change notification settings - Fork 51
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 common-types specification differences with azure-rest-api-specs #2250
base: main
Are you sure you want to change the base?
Fix common-types specification differences with azure-rest-api-specs #2250
Conversation
All changed packages have been documented.
Show changes
|
You can try these changes here
|
ValidatationCurrently under performance, still need to do some fixes
Fix to get in after merge |
@@ -6,34 +6,6 @@ | |||
}, | |||
"paths": {}, | |||
"definitions": { | |||
"CustomerManagedKeyEncryption": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced we want to make these changes to capitalization fo schema for a few reasons:
- They have no impact on the API on the wire, they only impact swagger references
- Clients should always use their own naming schemes for their particular language when naming common-types
I would want to see a compelling reason for capitalization-only changes in schema names
@@ -6,8 +6,8 @@ namespace Azure.ResourceManager; | |||
alias InfrastructureEncryption = CommonTypes.InfrastructureEncryption; | |||
alias KeyEncryptionIdentity = CommonTypes.KeyEncryptionKeyIdentity; | |||
alias KeyEncryptionKeyIdentity = CommonTypes.KeyEncryptionKeyIdentity; | |||
alias CustomerManagedKeyEncryption = CommonTypes.CustomerManagedKeyEncryption; | |||
alias EncryptionConfiguration = CommonTypes.Encryption; | |||
alias CustomerManagedKeyEncryption = CommonTypes.customerManagedKeyEncryption; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just reiterating the comment above. This is perpetuating a bug that does not impact the wire API
I will be probably doing a similar pr like this next sprint as there are still couple of differences that I am working trough, but this is some of the feedback provided in: Azure/azure-rest-api-specs#32562
TODO: Validate azure-rest-api-specs