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

TranscriptLoggerMiddleware bugfix #1261

Merged
merged 1 commit into from Jan 9, 2019
Merged

TranscriptLoggerMiddleware bugfix #1261

merged 1 commit into from Jan 9, 2019

Conversation

mark-szabo
Copy link
Contributor

Fixing a NullReferenceException bug which is thrown if the incoming Activity's From property is null in TranscriptLoggerMiddleware. Eg. if BotFrameworkAdapter.CreateConversationAsync() is triggering an 'event' Activity and for some reason does not sets the From property.

Fixing a NullReferenceException bug which is thrown if the incoming Activity's From property is null in `TranscriptLoggerMiddleware`. Eg. if `BotFrameworkAdapter.CreateConversationAsync()` is triggering an 'event' Activity and for some reason does not sets the From property.
@cleemullins
Copy link
Contributor

@mark-szabo, this looks great to me, but as it's a code change with new behavior, it needs a Unit Test that highlights the old/new behavior. Are you able to add that?

@cleemullins
Copy link
Contributor

@tomlm This looks great to me, but as it's your code I figured you should be the one to sign-off in case I'm missing something subtle.

@cleemullins cleemullins self-assigned this Jan 8, 2019
@cleemullins cleemullins added the bug Indicates an unexpected problem or an unintended behavior. label Jan 8, 2019
@mark-szabo
Copy link
Contributor Author

mark-szabo commented Jan 8, 2019

Hi @cleemullins sadly I don't have a dev env for botbuilder-dotnet, so it'd take time... If it's not fitting into your schedule, I'll do it as soon as I have time to set up a dev env and get familiar with your unit tests.

@cleemullins
Copy link
Contributor

Merging, per approval from @tomlm

@cleemullins cleemullins merged commit c901d04 into microsoft:master Jan 9, 2019
@mark-szabo mark-szabo deleted the patch-2 branch January 14, 2019 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or an unintended behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants