Skip to content
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

[Hub Generated] Review request for Microsoft.NotificationHubs to add version stable/2023-09-01 #25060

Merged

Conversation

kmiecikt
Copy link
Contributor

@kmiecikt kmiecikt commented Jul 31, 2023

This is a PR generated at OpenAPI Hub. You can view your work branch via this link.

ARM (Control Plane) API Specification Update Pull Request

PR review workflow diagram

Please understand this diagram before proceeding. It explains how to get your PR approved & merged.

diagram

Purpose of this PR

What's the purpose of this PR? Check all that apply. This is mandatory!

  • New API version. (Such PR should have been generated with OpenAPI Hub, per this wiki doc.)
  • Update existing version for a new feature. (This is applicable only when you are revising a private preview API version.)
  • Update existing version to fix swagger quality issues in S360.
  • Other, please clarify:
    • edit this with your clarification

Due diligence checklist

To merge this PR, you must go through the following checklist and confirm you understood
and followed the instructions by checking all the boxes:

ARM API changes review

  • If you want for the ARM team to review this PR, you must add the ARMReview label.
  • The automation may automatically add the ARMReview label, if appropriate.
    If this happens, proceed according to guidance given in GitHub comments also added by the automation.

Breaking change review

If you have any breaking changes as defined in the Breaking Change Policy,
follow the process outlined in the High-level Breaking Change Process doc.

Getting help

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jul 31, 2023

Swagger Validation Report

️️✔️BreakingChange succeeded [Detail] [Expand]
There are no breaking changes.
️❌Breaking Change(Cross-Version): 218 Errors, 34 Warnings failed [Detail]
compared swaggers (via Oad v0.10.4)] new version base version
notificationhubs.json 2023-09-01(bc6233a) 2017-04-01(main)
notificationhubs.json 2023-09-01(bc6233a) 2023-01-01-preview(main)

The following breaking changes are detected by comparison with the latest stable version:

Only 30 items are listed, please refer to log for more details.

Rule Message
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1003 - RequestBodyFormatNoLongerSupported The new version does not support 'application/json' as a request body format.
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L12:3
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.NotificationHubs/namespaces/{namespaceName}/AuthorizationRules/{authorizationRuleName}' removed or restructured?
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L316:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.NotificationHubs/namespaces/{namespaceName}/AuthorizationRules' removed or restructured?
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L547:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.NotificationHubs/namespaces/{namespaceName}/AuthorizationRules/{authorizationRuleName}/listKeys' removed or restructured?
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L594:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.NotificationHubs/namespaces/{namespaceName}/AuthorizationRules/{authorizationRuleName}/regenerateKeys' removed or restructured?
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L645:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.NotificationHubs/namespaces/{namespaceName}/notificationHubs/{notificationHubName}/AuthorizationRules/{authorizationRuleName}' removed or restructured?
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1037:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.NotificationHubs/namespaces/{namespaceName}/notificationHubs/{notificationHubName}/AuthorizationRules' removed or restructured?
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1263:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.NotificationHubs/namespaces/{namespaceName}/notificationHubs/{notificationHubName}/AuthorizationRules/{authorizationRuleName}/listKeys' removed or restructured?
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1317:5
1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.NotificationHubs/namespaces/{namespaceName}/notificationHubs/{notificationHubName}/AuthorizationRules/{authorizationRuleName}/regenerateKeys' removed or restructured?
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1375:5
1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'NamespaceCreateOrUpdateParameters' removed or renamed?
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L1879:3
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1494:3
1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'SharedAccessAuthorizationRuleCreateOrUpdateParameters' removed or renamed?
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L1879:3
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1494:3
1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'PolicykeyResource' removed or renamed?
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L1879:3
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1494:3
1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'NotificationHubCreateOrUpdateParameters' removed or renamed?
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L1879:3
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1494:3
1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'DebugSendParameters' removed or renamed?
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L1879:3
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L1494:3
1008 - ModifiedOperationId The operation id has been changed from 'Namespaces_Patch' to 'Namespaces_Update'. This will impact generated code.
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L938:7
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L173:7
1008 - ModifiedOperationId The operation id has been changed from 'NotificationHubs_Patch' to 'NotificationHubs_Update'. This will impact generated code.
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L181:7
Old: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json#L823:7
1011 - AddingResponseCode The new version adds a response code 'default'.
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L815:11
1011 - AddingResponseCode The new version adds a response code 'default'.
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L860:11
1011 - AddingResponseCode The new version adds a response code 'default'.
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L921:11
1011 - AddingResponseCode The new version adds a response code 'default'.
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L980:11
1011 - AddingResponseCode The new version adds a response code 'default'.
New: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L1023:11
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️❌LintDiff: 1 Errors, 0 Warnings failed [Detail]
compared tags (via openapi-validator v2.1.3) new version base version
package-2023-09 package-2023-09(bc6233a) default(main)

[must fix]The following errors/warnings are introduced by current PR:

Rule Message Related RPC [For API reviewers]
RepeatedPathInfo The 'subscriptionId' already appears in the path, please don't repeat it in the request body.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L886
RPC-Put-V1-05


The following errors/warnings exist before current PR submission:

Rule Message
OperationIdNounVerb Per the Noun_Verb convention for Operation Ids, the noun 'NotificationHubs' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L19
OperationsApiSchemaUsesCommonTypes Operations API path must follow the schema provided in the common types.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L1533
ArmResourcePropertiesBag Top level property names should not be repeated inside the properties bag for ARM resource 'NamespaceResource'. Properties [properties.name] conflict with ARM top level properties. Please rename these.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2601
ArmResourcePropertiesBag Top level property names should not be repeated inside the properties bag for ARM resource 'NotificationHubResource'. Properties [properties.name] conflict with ARM top level properties. Please rename these.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2761
TrackedResourcePatchOperation Tracked resource 'SharedAccessAuthorizationRuleResource' must have patch operation that at least supports the update of tags.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L3375
⚠️ PostOperationIdContainsUrlVerb OperationId should contain the verb: 'checknamespaceavailability' in:'Namespaces_CheckAvailability'. Consider updating the operationId
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L784
⚠️ PutInOperationName 'PUT' operation 'PrivateEndpointConnections_Update' should use method name 'Create'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L1560
⚠️ LocationMustHaveXmsMutability Property location must have 'x-ms-mutability':['read', 'create'] extension defined.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2120
⚠️ EnumInsteadOfBoolean Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2131
⚠️ EnumInsteadOfBoolean Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2149
⚠️ EnumInsteadOfBoolean Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2418
⚠️ EnumInsteadOfBoolean Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2519
⚠️ EnumInsteadOfBoolean Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2524
⚠️ EnumInsteadOfBoolean Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum.
Location: Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2796
️❌Avocado: 9 Errors, 0 Warnings failed [Detail]
Rule Message
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.NotificationHubs/namespaces/{}/AuthorizationRules/{} is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /{} is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2014-09-01/notificationhubs.json
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.NotificationHubs/namespaces/{}/AuthorizationRules is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.NotificationHubs/namespaces/{}/AuthorizationRules/{}/listKeys is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.NotificationHubs/namespaces/{}/notificationHubs/{}/AuthorizationRules/{} is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.NotificationHubs/namespaces/{}/notificationHubs/{}/AuthorizationRules is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.NotificationHubs/namespaces/{}/notificationHubs/{}/AuthorizationRules/{}/listKeys is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.NotificationHubs/namespaces/{}/AuthorizationRules/{}/regenerateKeys is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json
MISSING_APIS_IN_DEFAULT_TAG The default tag should contain all APIs. The API path /subscriptions/{}/resourceGroups/{}/providers/Microsoft.NotificationHubs/namespaces/{}/notificationHubs/{}/AuthorizationRules/{}/regenerateKeys is not in the default tag. Please make sure the missing API swaggers are in the default tag.
readme: specification/notificationhubs/resource-manager/readme.md
json: Microsoft.NotificationHubs/stable/2017-04-01/notificationhubs.json
️❌ApiReadinessCheck: 1 Errors, 0 Warnings failed [Detail]
Rule Message
API Readiness check failed. Please make sure your service is deployed. "code: InvalidResourceType,
message: The resource type 'operations' could not be found in the namespace 'Microsoft.NotificationHubs' for api version '2023-09-01'. The supported api-versions are '2014-09-01,
2016-03-01,
2017-04-01,
2020-01-01-preview'."
️⚠️~[NotRequired_Staging] ServiceAPIReadinessTest: 0 Warnings warning [Detail]

API Test is not triggered due to precheck failure. Check pipeline log for details.

️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️CadlAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️CadlValidation succeeded [Detail] [Expand]
Validation passes for CadlValidation.
️️✔️TypeSpec Validation succeeded [Detail] [Expand]
Validation passes for TypeSpec Validation.
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️Automated merging requirements met succeeded [Detail] [Expand]
Posted by Swagger Pipeline | How to fix these errors?

@openapi-workflow-bot
Copy link

Hi, @kmiecikt! Thank you for your pull request. To help get your PR merged:

  • Ensure you reviewed the checklists in the PR description.
  • Know that PR assignee is the person auto-assigned and responsible for your current PR review and approval.
  • For convenient view of the API changes made by this PR, refer to the URLs provided in the table in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.
  • @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented Jul 31, 2023

    Swagger Generation Artifacts

    ️️✔️ApiDocPreview succeeded [Detail] [Expand]

    Only 0 items are rendered, please refer to log for more details.

    ️❌SDK Breaking Change Tracking failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️❌ azure-sdk-for-net-track2 failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️⚠️ azure-sdk-for-python-track2 warning [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️⚠️ azure-sdk-for-java warning [Detail]

    Only 0 items are rendered, please refer to log for more details.

    ️️✔️ azure-sdk-for-go succeeded [Detail] [Expand]

    Only 0 items are rendered, please refer to log for more details.

    ️️✔️ azure-sdk-for-js succeeded [Detail] [Expand]

    Only 0 items are rendered, please refer to log for more details.

    ️️✔️ azure-resource-manager-schemas succeeded [Detail] [Expand]

    Only 0 items are rendered, please refer to log for more details.

    ️❌ azure-powershell failed [Detail]

    Only 0 items are rendered, please refer to log for more details.

    Posted by Swagger Pipeline | How to fix these errors?

    @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented Jul 31, 2023

    Generated ApiView

    Language Package Name ApiView Link
    Swagger Microsoft.NotificationHubs https://apiview.dev/Assemblies/Review/d346de9179c9458f8d97a78fec86b921
    Go sdk/resourcemanager/notificationhubs/armnotificationhubs https://apiview.dev/Assemblies/Review/7cf8e00b50f843d6b81ef97bfaeb98ba
    Java azure-resourcemanager-notificationhubs https://apiview.dev/Assemblies/Review/291a4592a89b47d4a8457421646c5f25
    JavaScript @azure/arm-notificationhubs https://apiview.dev/Assemblies/Review/25fe282fe4444f4dba269e87507b89e7

    @openapi-workflow-bot
    Copy link

    Hi @kmiecikt! The automation detected breaking changes in this pull request. As a result, it added the BreakingChangeReviewRequired label.

    You cannot proceed with merging this PR until you complete one of the following action items:

    ACTION ITEM ALTERNATIVE A: Fix the breaking change.
    Please consult the documentation provided in the relevant validation failures.

    ACTION ITEM ALTERNATIVE B: Request approval.
    Alternatively, if you cannot fix the breaking changes, then you can request an approval for them. Please follow the process described in the High-level Breaking Change Process doc.

    ACTION ITEM ALTERNATIVE C: Report false positive.
    If you think there are no breaking changes, i.e. the validation should pass yet it fails, then proceed as explained in ACTION ITEM ALTERNATIVE B.
    This applies even if the breaking change tool fails with internal runtime error. In such case a manual breaking change review is necessary.

    @openapi-workflow-bot
    Copy link

    Please address or respond to feedback from the ARM API reviewer.
    When you are ready to continue the ARM API review, please remove the ARMChangesRequested label.
    This will notify the reviewer to have another look.
    If the feedback provided needs further discussion, please use this Teams channel to post your questions - aka.ms/azsdk/support/specreview-channel.
    Please include [ARM Query] in the title of your question to indicate that it is ARM-related.

    @openapi-workflow-bot
    Copy link

    Hi @kmiecikt! Your PR has some issues. Please fix the CI issues, if present, in following order: Avocado, SemanticValidation, ModelValidation, Breaking Change, LintDiff.

    TaskHow to fixPriority
    AvocadoFix-AvocadoHigh
    Semantic ValidationFix-SemanticValidation-ErrorHigh
    Model ValidationFix-ModelValidation-ErrorHigh
    LintDiffFix-LintDiffHigh

    If you need further help, please reach out on the Teams channel aka.ms/azsdk/support/specreview-channel.

    @mikekistler
    Copy link
    Member

    Breaking changes previously reviewed and approved in #24072

    @mikekistler mikekistler added the Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 label Aug 3, 2023
    @kmiecikt
    Copy link
    Contributor Author

    @rkmanda: the stable version in this PR is exactly the same as already-approved API version 2023-01-01-preview (#24072).

    The linter / Avocado errors are either reported for previous versions (not updated in the PR), or also appeared in the previous PR and cannot be fixed without adding breaking changes.

    We will update the RP manifest next week. Is there any other action needed?

    @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented Aug 22, 2023

    Next Steps to Merge

    ⚠️ This is an experimental comment. It may not always be up-to-date. ⚠️

    ✔️ All automated merging requirements have been met! Refer to step 4 in the PR workflow diagram (even if your PR is for data plane, not ARM).

    @ms-henglu
    Copy link
    Member

    Please fix the Swagger Avocado and Swagger LintDiff.

    @ms-henglu ms-henglu added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Aug 22, 2023
    @openapi-workflow-bot openapi-workflow-bot bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 22, 2023
    @kmiecikt
    Copy link
    Contributor Author

    kmiecikt commented Aug 22, 2023

    Please fix the Swagger Avocado and Swagger LintDiff.

    @ms-henglu:

    The Swagger Avocado errors & warnings are incorrect - they are reported for older API version and files that did not change in this Pull Request.

    The remaining LintDiff errors cannot be fixed without introducing breaking changes. The same set of errors was already approved in the PR for 2023-01-01-preview.

    @kmiecikt kmiecikt added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 22, 2023
    @openapi-workflow-bot openapi-workflow-bot bot removed the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Aug 22, 2023
    @ms-henglu
    Copy link
    Member

    LintDiff previously reviewed and approved in #24072

    @ms-henglu ms-henglu added the ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review label Aug 22, 2023
    @openapi-workflow-bot openapi-workflow-bot bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 22, 2023
    @Alancere Alancere added the Approved-SdkBreakingChange-Go Approve the breaking change tracking for azure-sdk-for-go label Aug 23, 2023
    @kazrael2119
    Copy link
    Contributor

    this js sdk breaking is caused by base apiversion not this one.

    @kmiecikt
    Copy link
    Contributor Author

    Hi @rkmanda. Is there any other approval we need before completing the PR?

    @ms-henglu
    Copy link
    Member

    Hi @kmiecikt , please get the following errors fixed or approved.

    ❌ Swagger Avocado
    ❌ Approved-SdkBreakingChange-Python

    @kmiecikt
    Copy link
    Contributor Author

    Hi @kmiecikt , please get the following errors fixed or approved.

    ❌ Swagger Avocado ❌ Approved-SdkBreakingChange-Python

    Hi @ms-henglu.

    The Swagger Avocado errors are false positives - they are reported for files from previous version, not changes in this PR (please take a look at comments above).

    How can we approve the Python breaking changes? It was approved for version '2023-01-01-preview`, which is exactly the same as the stable one we try to add now.

    @ms-henglu
    Copy link
    Member

    Here's the list of sdk breakign changes and other checks approvers.

    @msyyc
    Copy link
    Member

    msyyc commented Aug 28, 2023

    Approved for python sdk breaking:
    image

    @kmiecikt
    Copy link
    Contributor Author

    @ms-henglu

    All reported errors / warnings were approved. Is there anything else we have to do before completing this PR?

    @ms-henglu
    Copy link
    Member

    @ms-henglu

    All reported errors / warnings were approved. Is there anything else we have to do before completing this PR?

    Please get the Avocado error approved label, you could find the contact from the list.

    @mentat9 mentat9 merged commit 64cddc5 into main Sep 6, 2023
    34 of 37 checks passed
    @mentat9 mentat9 deleted the kmiecikt-notificationhubs-Microsoft.NotificationHubs-2023-09-01 branch September 6, 2023 18:04
    jnlycklama pushed a commit that referenced this pull request Nov 8, 2023
    …version stable/2023-09-01 (#25060)
    
    * Adds base for updating Microsoft.NotificationHubs from version preview/2023-01-01-preview to version 2023-09-01
    
    * Updates readme
    
    * Updates API version in new specs and examples
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Approved-Avocado Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 Approved-LintDiff Approved-SdkBreakingChange-Go Approve the breaking change tracking for azure-sdk-for-go Approved-SdkBreakingChange-JavaScript Approved-SdkBreakingChange-Python ARMReview ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required CI-BreakingChange-Go CI-BreakingChange-JavaScript CI-FixRequiredOnFailure new-api-version resource-manager
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    10 participants