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.

Sign up for GitHub

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

Jump to bottom

Modifications to number of attempts property behavior #1774

Merged
merged 7 commits into from
Apr 24, 2019
Merged

Modifications to number of attempts property behavior #1774

merged 7 commits into from
Apr 24, 2019

Conversation

gabog
Copy link
Contributor

@gabog gabog commented Apr 24, 2019 •
edited

@johnataylor / @cleemullins: these changes address #1748. Please revuew and let me know what you think..

Changes include:

  • Removed NumberOfAttempts from PromptOptions
  • Added NumberOfAttempts getter to PromptValidatorContext
  • Implemented logic to keep internal count of number of attempts in the prompt state.
  • Updated unit test to compile and pass (this counter is 1 based)
  • Added missing cancellationTokens where needed

@gabog
First pass:
49ac6f5 
 - Removed NumberOfAttempts from PromptOptions
 - Added NumberOfAttempts getter to PromptValidatorContext
 - Implemented logic to keep internal count of number of attempts in the prompt state.
 - Updated unit test to pass
 - Added missing cancellationTokens where needed
@gabog
Changed the counter to start at 1.
f0d4d5e 
Changed documentation for getter.
Updated test to show the try count on succes.
@gabog gabog changed the title Gabog/1748 number of attempts property Modifications to number of attempts property behavior Apr 24, 2019
@fuselabs
Copy link
Collaborator

@fuselabs
Copy link
Collaborator

@johnataylor johnataylor mentioned this pull request Apr 24, 2019
Merged
@fuselabs
Copy link
Collaborator 

No issues found in Microsoft.Bot.Builder.dll
No issues found in Microsoft.Bot.Builder.AI.Luis.dll
No issues found in Microsoft.Bot.Builder.AI.QnA.dll
No issues found in Microsoft.Bot.Builder.ApplicationInsights.dll
No issues found in Microsoft.Bot.Builder.Azure.dll
No issues found in Microsoft.Bot.Builder.Dialogs.dll
No issues found in Microsoft.Bot.Builder.TemplateManager.dll
No issues found in Microsoft.Bot.Configuration.dll
No issues found in Microsoft.Bot.Connector.dll
No issues found in Microsoft.Bot.Schema.dll

@gabog gabog marked this pull request as ready for review April 24, 2019 19:54
Stevenic
Stevenic approved these changes Apr 24, 2019
View reviewed changes
Copy link
Contributor

@Stevenic Stevenic left a comment

Choose a reason for hiding this comment

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

:shipit:

@gabog gabog dismissed johnataylor’s stale review April 24, 2019 20:25

Already addressed

@gabog
Update PromptValidatorContext.cs
2448abb 
Touched to force a build
@fuselabs
Copy link
Collaborator 

No issues found in Microsoft.Bot.Builder.dll
No issues found in Microsoft.Bot.Builder.AI.Luis.dll
No issues found in Microsoft.Bot.Builder.AI.QnA.dll
No issues found in Microsoft.Bot.Builder.ApplicationInsights.dll
No issues found in Microsoft.Bot.Builder.Azure.dll
No issues found in Microsoft.Bot.Builder.Dialogs.dll
No issues found in Microsoft.Bot.Builder.TemplateManager.dll
No issues found in Microsoft.Bot.Configuration.dll
No issues found in Microsoft.Bot.Connector.dll
No issues found in Microsoft.Bot.Schema.dll

@gabog gabog merged commit 22a63db into master Apr 24, 2019
@gabog gabog deleted the gabog/1748_numberOfAttemptsProperty branch April 24, 2019 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Stevenic Stevenic Stevenic approved these changes

@cleemullins cleemullins Awaiting requested review from cleemullins

@johnataylor johnataylor Awaiting requested review from johnataylor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@gabog @fuselabs @Stevenic @johnataylor