-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Added the replica count API change to Shared Image Gallery swagger file. #3757
Added the replica count API change to Shared Image Gallery swagger file. #3757
Conversation
Automation for azure-sdk-for-rubyNothing to generate for azure-sdk-for-ruby |
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-nodeA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
Automation for azure-sdk-for-javaNothing to generate for azure-sdk-for-java |
Automation for azure-sdk-for-goThe initial PR has been merged into your service PR: |
"type": "string", | ||
"description": "The name of the region." | ||
}, | ||
"regionalReplicaCount": { |
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.
If the name of the region is called name
, why the replica count still has the region
as the property's prefix? Why not just replicaCount
?
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.
Yes. Initially, we call it "replicaCount", the same as you suggested. Then we got some feedback from Kay Singh that it might be clearer to use "regionalReplicaCount". So we use this.
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.
OK
"regionalReplicaCount": { | ||
"type": "integer", | ||
"format": "int32", | ||
"description": "This is the number of source blob copies in this specific region." |
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.
source blob
in the source region, or in the target region? Or target blob
? Please consider revising the statement to make it clearer.
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.
So it is the copies of the source blob in the target regions. Will this be better "This is the number of copies of the source blobs in this target region."?
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.
OK
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.
@vanbasten2323 what exactly is the "number of copies of the source blobs mean"?
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.
@singhkays, the original mail communicates the new name would be scaleTier
, so i am bit confused now. Can you guys offer some explanation, so i can communicate better to CLI users.
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.
Also, what is the service end default value for this?
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.
Clarified in offline call
LGTM, with some minor comments. |
Can one of the admins verify this patch? |
This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
PR information
api-version
in the path should match theapi-version
in the spec).Quality of Swagger