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

There are many new async methods that don't have the Async suffix in their name #3889

Closed
JonathanFingold opened this issue May 8, 2020 · 2 comments · Fixed by #3890
Closed

There are many new async methods that don't have the Async suffix in their name #3889

JonathanFingold opened this issue May 8, 2020 · 2 comments · Fixed by #3890
Assignees
@carlosscastro
Labels
P0 Must Fix. Release-blocker R9 Release 9 - May 15th, 2020
Milestone
R9

Comments

@JonathanFingold
Copy link
Contributor

JonathanFingold commented May 8, 2020 •
edited
Loading

Github issues should be used for bugs and feature requests. Use Stack Overflow for general "how-to" questions.

Version

What package version of the SDK are you using. master (as of 5/8/2020 AM)

Describe the bug

Our internal guidelines are to always suffix async methods with Async. The new methods for this release are inconsistent on that front.

To Reproduce

Steps to reproduce the behavior:

  1. Search for async methods in the adaptive dialogs library and notice that there are many that are missing an Async suffix.
  2. For example:

Expected behavior

We follow our own guidelines consistently and this is part of code review, especially for anything that represents a breaking change to fix.

Additional context

Some number of async methods also lack the optional CancellationToken parameter.

[bug]
@cleemullins cleemullins assigned tomlm and carlosscastro and unassigned tomlm May 8, 2020
@cleemullins cleemullins added P0 Must Fix. Release-blocker R9 Release 9 - May 15th, 2020 labels May 8, 2020
@cleemullins
Copy link
Contributor

@carlosscastro - thanks for taking this today!

@stevengum stevengum mentioned this issue May 8, 2020
Closed
@carlosscastro carlosscastro mentioned this issue May 8, 2020
Merged
@cleemullins cleemullins reopened this May 8, 2020
@stevengum stevengum mentioned this issue May 8, 2020
Merged
@sgellock
Copy link
Member

sgellock commented May 9, 2020

PR #3892 was merged

@sgellock sgellock closed this as completed May 9, 2020
@munozemilio munozemilio added this to the R9 milestone Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carlosscastro carlosscastro

Labels
P0 Must Fix. Release-blocker R9 Release 9 - May 15th, 2020
Projects
None yet
Milestone
R9
Development

Successfully merging a pull request may close this issue.

Merge 4.9 into master microsoft/botbuilder-dotnet
6 participants
@sgellock @cleemullins @tomlm @carlosscastro @JonathanFingold @munozemilio