Skip to content

V10#599

Merged
mcardosos merged 6 commits into
Azure:devfrom
mcardosos:v10
May 4, 2017
Merged

V10#599
mcardosos merged 6 commits into
Azure:devfrom
mcardosos:v10

Conversation

@mcardosos

Copy link
Copy Markdown
Contributor

Comment thread arm/apimanagement/apiexport.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this funky casing due to swagger authoring or a bug in the generator?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is because of the PR I submitted to Autorest

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This last commit was not generated from master, but from my local branch)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, but shouldn't this be APIID?

Comment thread Gododir/gen.go
@@ -504,9 +534,9 @@ func generate(service *service) {
fmt.Printf("Generating %s...\n\n", service.Fullname)
delete(service)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get rid o' this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

//
// resourceGroupName is the name of the resource group. serviceName is the name
// of the API Management service. apiID is aPI identifier. Must be unique in
// of the API Management service. aPIID is aPI identifier. Must be unique in

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, is there anything we can do about this? It definitely seems regressive.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you prefer APIID ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I actually prefer apiID but I'm not sure if there's anyway to get that back without causing further breaking changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason this appeared is because of this... Azure/autorest#2220 (just for reference)
In this PR, travis failed in the first commit with this message...
arm/hdinsight/models.go:534:2: struct field ClusterUsersGroupDNs should be ClusterUsersGroupDNS

// endswith | top is number of records to return. skip is number of records to
// skip.
func (client APIProductsClient) ListByAPI(resourceGroupName string, serviceName string, apiID string, filter string, top *int32, skip *int32) (result ProductCollection, err error) {
func (client APIProductsClient) ListByApis(resourceGroupName string, serviceName string, aPIID string, filter string, top *int32, skip *int32) (result ProductCollection, err error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to callout this breaking change

Comment thread arm/apimanagement/apis.go
// Contract parameters. ifMatch is eTag of the API entity. ETag should match
// the current entity state in the header response of the GET request or it
// should be * for unconditional update.
func (client ApisClient) Update(resourceGroupName string, serviceName string, aPIID string, parameters APIUpdateContract, ifMatch string) (result autorest.Response, err error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PatchParameters -> APIUpdateContract would be another good thing to call out.

Comment thread CHANGELOG.md Outdated
| arm/commerce | 2015-06-01-preview | refactor |
| arm/compute | 2016-04-30-preview | refactor |
| arm/consumption | 2017-04-24-preview | new |
| arm/containerregistry | 2017-06-01-preview | update to latest swagger & refactor |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This api-version is still under private preview and the SDK is not supposed to be published yet. Can we keep the 2017-03-01 version? /cc @sajayantony

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will regen for 2017-03-01 then :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. What is the release pattern/process from dev to master? We have recently noticed a large number of calls from Go SDK to 2016-06-27-preview since it is the version in master branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each time we release we merge dev to master. We release every month.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks for noticing this.

@mcardosos mcardosos merged commit 0308f8f into Azure:dev May 4, 2017
mcardosos added a commit that referenced this pull request May 4, 2017
* Gen everything

* Fix golint issue

* Changelog and review

* Updated readme and changelog

* Gen correct APIver of container registry

* Added sample link to the readme
@mcardosos mcardosos deleted the v10 branch June 27, 2017 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants