[ACR] Update to 2017-03-01 version#2946
Conversation
1. Update getCredentials to listCredentials to support multiple login credentials. 2. Refine regenerateCredential to support regenerate the specified login credential. 3. Add Sku to registry properties as a required property. 4. Rename GetProperties to Get. 5. Change CreateOrUpdate to Create, add registry create parameters.
|
/cc @sajayantony |
|
/cc @JasonRShaver can we get this merged. RPs have been already updated. |
cormacpayne
left a comment
cormacpayne
left a comment
There was a problem hiding this comment.
@djyou a couple of comments that need to be resolved (the comments that point out the breaking changes are just for identification - no action needed with those)
| } | ||
|
|
||
| [Fact] | ||
| public void ContainerRegistryUpdateWithCreateTest() |
There was a problem hiding this comment.
We changed CreateOrUpdate to Create in this api-version.
| set autoRestVersion=1.0.0-Nightly20170212 | ||
| if "%1" == "" ( | ||
| set specFile="https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-containerregistry/2016-06-27-preview/swagger/containerregistry.json" | ||
| set specFile="https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-containerregistry/2017-03-01/swagger/containerregistry.json" |
| @@ -1,48 +0,0 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
There was a problem hiding this comment.
Yes, this is a new api-version with breaking change.
| [Newtonsoft.Json.JsonProperty(PropertyName = "properties.storageAccount")] | ||
| public StorageAccountProperties StorageAccount { get; set; } | ||
| [JsonProperty(PropertyName = "properties.storageAccount")] | ||
| public StorageAccountParameters StorageAccount { get; set; } |
There was a problem hiding this comment.
@djyou changing the name of a property in a model class is a breaking change
There was a problem hiding this comment.
Yes, this is a new api-version with breaking change.
| /// Gets or sets the access key to the storage account. | ||
| /// </summary> | ||
| [Newtonsoft.Json.JsonProperty(PropertyName = "accessKey")] | ||
| public string AccessKey { get; set; } |
There was a problem hiding this comment.
@djyou removing the property of a model class is a breaking change
There was a problem hiding this comment.
Yes, this is a new api-version with breaking change.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "version": "1.1.0-preview", | |||
| "version": "1.2.0", | |||
There was a problem hiding this comment.
@djyou since you are introducing breaking changes, this will need to remain in preview
There was a problem hiding this comment.
Our service is going GA as of this 2017-03-01 api-version, do we still keep the SDK as preview?
There was a problem hiding this comment.
Yes, we have a concept of preview vs stable SDKs: when a breaking change is introduced (or a large feature is added) to the SDK, the package must go into (or stay) preview for X amount of time, after which the package can be considered stable and the -preview suffix can be removed from the version. We are trying to give customers an idea of the stability of the packages they are using from this repository.
There was a problem hiding this comment.
Thank you for explaining. Fixed.
|
@azuresdkci test this please |
|
@cormacpayne Are there more comments or can we get this merged so we can publish the package? Thanks. |
4 participants
Generated from swagger https://github.com/Azure/azure-rest-api-specs/blob/master/arm-containerregistry/2017-03-01/swagger/containerregistry.json
Description
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
project.jsonandAssemblyInfo.csfiles have been updated with the new version of the SDK.