-
Notifications
You must be signed in to change notification settings - Fork 479
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
[WebexAdapter] Apply feedback to have adapter ready for release #2770
Conversation
Pull Request Test Coverage Report for Build 84827
💛 - Coveralls |
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.5.3 |
1 similar comment
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.5.3 |
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.5.3 |
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.
Two changes required....
- I have added a comment on the method where we should be throwing an exception instead of returning null.
- Please update the attachment logic to ignore additional attachments instead of throwing an exception. As a minimum we will take the first attachment and ignore the rest. Bonus points if you can simply find the first webex compatible attachment (in case the first attachment isn't the right type for webex, but the second one is).
- Bot.Schema - Bot.Builder
1202dc2
to
3f8ff81
Compare
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.5.3 |
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.
@ceciliaavila Thanks for making those changes. This is looking good.
Description
Applied minor changes in order to even all the adapters and getting them ready for release.
Details