[Communication]Acs chat preview6 - #19232
Conversation
ghost
added
the
luc (LuChen-Microsoft)
changed the title
Rajarshi Sarkar (sarkar-rajarshi)
left a comment
Rajarshi Sarkar (sarkar-rajarshi)
left a comment
There was a problem hiding this comment.
- Minor updates required with docstrings.
- Minor updates required in the samples
Also, do we not need to update the live tests?
| self, | ||
| message_id: str, | ||
| content: str = None, | ||
| metadata: dict[str, str] = None, |
There was a problem hiding this comment.
missing docstring
|
|
||
| def setUp(self): | ||
| super(CommunicationTestCase, self).setUp() | ||
| print(111111) |
There was a problem hiding this comment.
Does this value have any signficance?
Rajarshi Sarkar (sarkar-rajarshi)
left a comment
Rajarshi Sarkar (sarkar-rajarshi)
left a comment
There was a problem hiding this comment.
luc (@LuChen-Microsoft) Please add tests for the metadata changes for the live tests as well. Other than that, looks good.
| self, | ||
| message_id, # type: str | ||
| content=None, # type: Optional[str] | ||
| metadata=None, # type: Optional[dict[str, str]] |
There was a problem hiding this comment.
Let's move this into kwargs, and document it as keyword only (same as we did for send_message)
There was a problem hiding this comment.
Anna Tisch (@annatisch) do we need to also put content in kwargs? it's also optional.
There was a problem hiding this comment.
Depends on whether this library has already has a GA 1.0 release. If so - to move content into kwargs would be a breaking change.
| :paramtype chat_message_type: Union[str, ~azure.communication.chat.ChatMessageType] | ||
| :keyword str sender_display_name: The display name of the message sender. This property is used to | ||
| populate sender name for push notifications. | ||
| :keyword dict[str, str] metadata : Message metadata. |
There was a problem hiding this comment.
I don't see any updates to the SDK version - I presume this will be shipping as a new preview?
Also, could you please add some details to the changelog?
| :type message_id: str | ||
| :param content: Chat message content. | ||
| :type content: str | ||
| :param metadata: Message metadata. |
There was a problem hiding this comment.
This will need to be documented as keyword rather than param:
:keyword dict[str, str] metadata : Message metadata.
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| ## 1.0.1 (Unreleased) | |||
There was a problem hiding this comment.
This needs to be bumped to 1.1.0
|
/azp run python - communication - tests |
|
No pipelines are associated with this pull request. |
|
/azp run python - azure-communication-chat - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - azure-communication-chat - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - azure-communication-chat - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - azure-communication-chat - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
No description provided.