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

Fix class structure for meeting notification feature extensibility #6579

Merged
merged 4 commits into from
Feb 10, 2023

Conversation

yingduyingdu
Copy link
Collaborator

@yingduyingdu yingduyingdu commented Jan 19, 2023

Fixes #6580

Description

This PR fixes the extensibility issue for the Teams bot meeting notification feature by updating on model classes.

Specific Changes

Update classes to support different types of meeting notification, surface type, and content type.
Update reference and unit tests.

Testing

Update and pass unit tests.
Pass manual testing via personal test bot.

@yingduyingdu yingduyingdu force-pushed the yingdu/fixClass branch 5 times, most recently from 2779c8c to 6ab06ca Compare January 23, 2023 21:16
@yingduyingdu yingduyingdu marked this pull request as ready for review January 23, 2023 21:17
@yingduyingdu yingduyingdu requested a review from a team as a code owner January 23, 2023 21:17
Copy link

@YunnyChung YunnyChung left a comment

Choose a reason for hiding this comment

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

First PR review :)

libraries/Microsoft.Bot.Builder/Teams/TeamsInfo.cs Outdated Show resolved Hide resolved
libraries/Microsoft.Bot.Schema/Teams/SurfaceType.cs Outdated Show resolved Hide resolved
libraries/Microsoft.Bot.Schema/Teams/Surface.cs Outdated Show resolved Hide resolved
libraries/Microsoft.Bot.Schema/Teams/Surface.cs Outdated Show resolved Hide resolved
Copy link

@YunnyChung YunnyChung left a comment

Choose a reason for hiding this comment

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

Additional comment for my own learning :)

@yingduyingdu yingduyingdu force-pushed the yingdu/fixClass branch 2 times, most recently from 66bbed4 to c7e4058 Compare January 26, 2023 05:15
Copy link

@YunnyChung YunnyChung left a comment

Choose a reason for hiding this comment

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

A few follow ups

@yingduyingdu yingduyingdu requested review from vule-msft and removed request for YunnyChung January 30, 2023 21:14
@yingduyingdu yingduyingdu force-pushed the yingdu/fixClass branch 2 times, most recently from 7999537 to 8f29579 Compare January 31, 2023 19:14
@BruceHaley
Copy link
Contributor

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll
✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll
✔️ No Binary Compatibility issues for Microsoft.Bot.Schema.dll

Copy link

@YunnyChung YunnyChung left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you :)

@tracyboehrer tracyboehrer removed the request for review from vule-msft February 10, 2023 20:11
@tracyboehrer tracyboehrer merged commit ce40a2d into main Feb 10, 2023
@tracyboehrer tracyboehrer deleted the yingdu/fixClass branch February 10, 2023 20:11
tracyboehrer pushed a commit that referenced this pull request Feb 13, 2023
…6579)

* draft PR check remote build

* Fix class model, update reference, unit test, and comments. Unit tests pass, manual tests pass

* Addressing comments

---------

Co-authored-by: Ying Du <[email protected]>
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.

Fix class structure for teams bot meeting notification to resolve breaking change when extending surface type
6 participants