-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[2019-10-01-preview] Fixes and additional improvements for Public Preview #10382
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
Conversation
|
Azure Pipelines successfully started running 1 pipeline(s). |
azure-sdk-for-java - Release
|
azure-sdk-for-go - Release
|
Azure CLI Extension Generation - Release⌛ Pending...
|
azure-sdk-for-python - Release
|
|
Can one of the admins verify this patch? |
Trenton Generation - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
azure-sdk-for-net - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
azure-sdk-for-python-track2 - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
6fe592c to
5162774
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
5162774 to
2a25935
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
2a25935 to
653938d
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
653938d to
4459e90
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
4459e90 to
342301c
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
342301c to
b918654
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Azure Pipelines successfully started running 1 pipeline(s). |
a058510 to
6520fe0
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
6520fe0 to
ded8339
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
PhoenixHe-NV
left a comment
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.
Your PR contains a lot of breaking change. Please try to avoid breaking change or please provide reason for each of them
| "provisioningState": { | ||
| "$ref": "#/definitions/ProvisioningState" | ||
| "$ref": "#/definitions/ProvisioningState", | ||
| "readOnly": true |
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.
This will not work. If you want to modify the readOnly, please consider adding another model
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.
do you mean this is breaking change and hence not allowed to change it to readonly?
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 mean it will not work to add "readOnly" on a ref directly. Only comment and description are allowed on ref
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.
got it, i have updated the referred definition to readonly.
...move/resource-manager/Microsoft.Migrate/preview/2019-10-01-preview/regionmovecollection.json
Show resolved
Hide resolved
ded8339 to
7a7b66b
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
azure-resource-manager-schemas - Release
|
7a7b66b to
7033ff0
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
1. Support to pass source id in Prepare/Move/Commit/Discard operations. 2. Define the additinal error information schema. 3. Expose the read only source settings. 4. Few fixes in marking the properties readonly/mandatory as appropriate.
7033ff0 to
91cbf78
Compare
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Breaking Changes Details
Summary of breaking change:
Bugs in swagger:
Explanations for the breaking change in detail:
|
|
@innosam Your go generation failed. @ArcturusZhang for more comments |
|
Hi to resolve the go SDK generation issue, could you please also change the namespace and output folder in this file? |
|
Thanks @ArcturusZhang and @innosam . Merge. |
1. Support to pass source id in Prepare/Move/Commit/Discard operations. 2. Define the additinal error information schema. 3. Expose the read only source settings. 4. Few fixes in marking the properties readonly/mandatory as appropriate. Co-authored-by: Harkirat Singh <[email protected]>
appropriate.
### Breaking Changes
This is a Preview PR and the breaking change policy does not apply. Guidelines-and-Scope
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Contribution checklist:
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Ensure to check this box if one of the following scenarios meet updates in the PR, so that label “WaitForARMFeedback” will be added automatically to involve ARM API Review. Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs, all “removals” and “adding a new property” no more require ARM API review.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If there are following updates in the PR, ensure to request an approval from API Review Board as defined in the Breaking Change Policy.
Please follow the link to find more details on PR review process.