-
Notifications
You must be signed in to change notification settings - Fork 281
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
move numberOfAttempts to state from options #889
Conversation
johnataylor
commented
Apr 24, 2019
- moves the numberOfAttempts property into the state from the options
- adds a typed/named property to the top level for parity with the typing/discovery C# implementation
also refer to microsoft/botbuilder-dotnet#1774 |
is defaulting it to 0 the right thing to do? |
Pull Request Test Coverage Report for Build #2222
💛 - Coveralls |
Its interesting you should comment on that - the original was 0 based - the reason being one of the natural pieces of code to write was to look up the prompt test in an array and arrays are 0 based. Anyhow I'm not that strongly of an opinion on this, but i think the 0 based array is something to consider. I mean this actually is an index. (Unlike JavaScript month which is 0 based but isn't 😱 ) |
…ub.com/Microsoft/botbuilder-js into johtaylo/1748_numberOfAttemptsProperty
The tests are failing. Build error is:
|