Skip to content

Azure Storage 2018-02-01#2937

Merged
jianghaolu merged 8 commits intoAzure:masterfrom
JasonYang-MSFT:containerapi2018-02-01
Apr 26, 2018
Merged

Azure Storage 2018-02-01#2937
jianghaolu merged 8 commits intoAzure:masterfrom
JasonYang-MSFT:containerapi2018-02-01

Conversation

@JasonYang-MSFT
Copy link
Member

@JasonYang-MSFT JasonYang-MSFT commented Apr 24, 2018

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.

Related PR: https://github.com/Azure/azure-rest-api-specs-pr/pull/447

PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes.
  • Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

@AutorestCI
Copy link

AutorestCI commented Apr 24, 2018

Automation for azure-libraries-for-java

The initial PR has been merged into your service PR:
AutorestCI/azure-libraries-for-java#114

@AutorestCI
Copy link

AutorestCI commented Apr 24, 2018

Automation for azure-sdk-for-node

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-node#2796

@azuresdkciprbot
Copy link

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(2 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@azuresdkciprbot
Copy link

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@AutorestCI
Copy link

AutorestCI commented Apr 24, 2018

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#2471

@AutorestCI
Copy link

AutorestCI commented Apr 24, 2018

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#1703

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason of adding these? We already have some of these common types defined here: https://github.com/Azure/azure-rest-api-specs/blob/master/specification/common-types/resource-management/v1/types.json

Copy link
Member

Choose a reason for hiding this comment

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

Fixed. And move the AzureEntityResource to type.json.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Fixed. But since the "ResourceGroupNameParameter" defined in https://github.com/Azure/azure-rest-api-specs/blob/master/specification/common-types/resource-management/v1/types.json, has "x-ms-parameter-location": "client", which will take breaking change to storage SDK, I define it in storage.json. We can revert to use common type when the definition difference of "ResourceGroupNameParameter" is resolved.

@blueww blueww force-pushed the containerapi2018-02-01 branch from 5e65907 to 45ac137 Compare April 25, 2018 06:07
@azuresdkciprbot
Copy link

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@azuresdkciprbot
Copy link

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@blueww
Copy link
Member

blueww commented Apr 25, 2018

@jianghaolu
I have fixed as your comments. But the "ResourceGroupNameParameter" in type.json is defined different with origin one on "x-ms-parameter-location": "client", which will take breaking to storage SDK.
So I don't use it in type.json, but define in storage.json.

Since the release of this feature is in a tight schedule, I will setup a meeting tomorrow in case the PR still has some comments need to follow.

@blueww blueww force-pushed the containerapi2018-02-01 branch from 45ac137 to d3205ab Compare April 25, 2018 07:01
@azuresdkciprbot
Copy link

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(0 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@azuresdkciprbot
Copy link

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(0 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@blueww blueww force-pushed the containerapi2018-02-01 branch from d3205ab to e8f30fe Compare April 25, 2018 07:06
@azuresdkciprbot
Copy link

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@azuresdkciprbot
Copy link

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@blueww
Copy link
Member

blueww commented Apr 25, 2018

@jianghaolu
The build check failed. But seems not caused by my code change?
Could you please help to look?

@blueww blueww force-pushed the containerapi2018-02-01 branch from e8f30fe to c1888cc Compare April 25, 2018 09:00
@azuresdkciprbot
Copy link

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@azuresdkciprbot
Copy link

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

…e the common one has input it from client, which is breaking
@blueww blueww force-pushed the containerapi2018-02-01 branch from c1888cc to 63f725f Compare April 25, 2018 09:23
@azuresdkciprbot
Copy link

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@azuresdkciprbot
Copy link

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/storage/resource-manager/readme.md

⚠️0 new Warnings.(3 total)
0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

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.

6 participants