-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix Communication Rooms Readme Issue And Push Recordings In Communication Library #28607
Conversation
6f00505
to
02104bf
Compare
Hi @v-weiyding. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
Hi @v-weiyding. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing |
API change check API changes are not detected in this pull request. |
this commit ix communication-message-rest ci test error is just update the recording to fix following ci error |
for this commit fix communication-email ci test error is to fix |
for this commit fix communication-job-router ci test error is to fix |
for this commit fix communication-job-router-rest ci test error is to fix |
this commit |
...s_lists_and_deletes_us_program_brief/recording_can_create_and_delete_a_us_program_brief.json
Show resolved
Hide resolved
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.
LGTM
@xirzec since this pr is updating communication data plane libraries, do you have any concerns here? |
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.
Left one minor note where I think we could avoid some test assertion changes by removing a central sanitizer like in other packages.
Overall, the changes look good and I'm excited to get more recordings out of the repo. @akania @DominikMe @allchiang-msft any thoughts/feedback before this gets merged?
sdk/communication/communication-phone-numbers/test/public/siprouting/getRoutes.spec.ts
Outdated
Show resolved
Hide resolved
I will merge this PR, @akania @DominikMe @allchiang-msft feel free to let me know if you have any concerns. |
Fixes #27565
@joheredi, @mayurid, @alexokun for notification.