API spec changes for PSTN Dialout for Rooms#25451
Conversation
shwali-msft
requested review from
JeffreyRichter and
markweitzel
and removed request for
a team
Next Steps to Merge✔️ 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). |
Swagger Validation Report
|
| compared swaggers (via Oad v0.10.4)] | new version | base version |
|---|---|---|
| communicationservicesrooms.json | 2023-10-30-preview(cf38c10) | 2023-06-14(main) |
| communicationservicesrooms.json | 2023-10-30-preview(cf38c10) | 2023-03-31-preview(main) |
The following breaking changes are detected by comparison with the latest stable version:
The following breaking changes are detected by comparison with the latest preview version:
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️LintDiff: 3 Warnings warning [Detail]
| compared tags (via openapi-validator v2.1.4) | new version | base version |
|---|---|---|
| package-rooms-2023-10-30-preview | package-rooms-2023-10-30-preview(cf38c10) | default(main) |
[must fix]The following errors/warnings are introduced by current PR:
| Rule | Message | Related RPC [For API reviewers] |
|---|---|---|
| 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: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L409 |
||
| 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: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L509 |
||
| 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: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L594 |
The following errors/warnings exist before current PR submission:
| Rule | Message |
|---|---|
| header parameter name 'Repeatability-Request-ID' should be kebab case. Location: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L28 |
|
| Using post for a create operation is discouraged. Location: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L51 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L132 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L182 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L238 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L284 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L338 |
|
| Schema should have a description or title. Location: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L431 |
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️❌ModelValidation: 2 Errors, 0 Warnings failed [Detail]
| Rule | Message |
|---|---|
|
Expected type but found type object Url: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L358:22 ExampleUrl: preview/2023-10-30-preview/examples/Participants_Update.json#L18:16 |
|
Expected type but found type object Url: Rooms/preview/2023-10-30-preview/communicationservicesrooms.json#L358:22 ExampleUrl: preview/2023-10-30-preview/examples/Participants_Update.json#L18:16 |
️️✔️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).
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️Automated merging requirements met succeeded [Detail] [Expand]
|
Swagger pipeline restarted successfully, please wait for status update in this comment. |
Generated ApiView
|
AzureRestAPISpecReview
added
BreakingChangeReviewRequired
|
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
|
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
|
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
|
I see new required properties here: https://github.com/Azure/azure-rest-api-specs/pull/25451/checks?check_run_id=16123854044 |
|
Adding the new flag property is fine but the description is not great. How about: Set this flag to true if, at the time of the call, dial out to a PSTN number in enabled in a particular room. |
|
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
|
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
Updated description as suggested. |
Yes, this needs to be added as a required property in RoomModel object. I have scheduled an API review with ARB for getting this reviewed. |
| "example": "2021-09-08T15:55:41Z" | ||
| }, | ||
| "enablePstnDialout": { | ||
| "description": "Set this flag to true if, at the time of the call, dial out to a PSTN number in enabled in a particular room.", |
There was a problem hiding this comment.
| "description": "Set this flag to true if, at the time of the call, dial out to a PSTN number in enabled in a particular room.", | |
| "description": "Set this flag to true if, at the time of the call, dial out to a PSTN number is enabled in a particular room.", |
JeffreyRichter
added
the
Approved-BreakingChange
JeffreyRichter
added
the
APIStewardshipBoard-SignedOff
|
Approving the model validation, as the same were identified as false positive and confirmed with Azure Board in an earlier pr #22540 |
anuchandy
added
Model Validation
Approved-ModelValidation
and removed
Model Validation
labels
|
Swagger pipeline restarted successfully, please wait for status update in this comment. |
* Changes for PSTN dialout review * Adding examples * Modifying Examples jsons * Adding version in Readme.md * correcting version in comment * adding Dialout to words * nit * Resolving review comments * nits * renaming enablePstnDialout to enablePstnDialOut * Changing name from enablePstnDialOut to pstnDialOutEnabled as per review feedback
6 participants
API spec changes for PSTN Dialout for Rooms
fix #25486