Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

port: Johtaylo/dialogrunasync (#5294) #1046

Closed
github-actions bot opened this issue Mar 11, 2021 · 0 comments · Fixed by #1059
Closed

port: Johtaylo/dialogrunasync (#5294) #1046

github-actions bot opened this issue Mar 11, 2021 · 0 comments · Fixed by #1059
Assignees
Labels
ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report. parity The issue describes a gap in parity between two or more platforms.

Comments

@github-actions
Copy link

The changes in Johtaylo/dialogrunasync (#5294) may need to be ported to maintain parity with microsoft/botbuilder-dotnet.

Fixes #5293

This fix removes the duplicate code. It follows the suggestion made in the associated issue to share an internal implementation.

The main reason for this is because the DialogManager is expected to return a DialogTurnResult. RunAsync never returned this, because the very point of the function was to hide this, and the associated continue-begin dance, as it is meaningless to the caller. (Note when use directly in an IBot implementation the result is, naturally, ignored.)

The other, even more obscure, reason is that the DialogManager exposes indirectly the DialogSet the DialogContext uses. As it is, it seems hard to predict whether an application could make use of a potential side effect of that leak in the abstraction. Making this change this way with the internal function doesn't impact that as the internal function takes an already created DialogContext.

Please review and, if necessary, port the changes.

@github-actions github-actions bot added ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report. needs-triage The issue has just been created and it has not been reviewed by the team. parity The issue describes a gap in parity between two or more platforms. labels Mar 11, 2021
@tracyboehrer tracyboehrer modified the milestone: R13 Mar 15, 2021
@tracyboehrer tracyboehrer removed the needs-triage The issue has just been created and it has not been reviewed by the team. label Mar 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ExemptFromDailyDRIReport Use this label to exclude the issue from the DRI report. parity The issue describes a gap in parity between two or more platforms.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants