Skip to content

Enable VSTHRD200 (Use "Async" suffix for async methods) #4794

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

Merged
merged 1 commit into from
Mar 2, 2020

Conversation

sharwell
Copy link
Contributor

@sharwell sharwell commented Feb 6, 2020

This rule relies on a suppression analyzer (#4803) to avoid requiring test methods to be renamed.

Copy link
Contributor

@harishsk harishsk left a comment

Choose a reason for hiding this comment

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

:shipit:

@@ -47,7 +47,7 @@ public interface IAsyncSweeper
/// Propose a <see cref="ParameterSet"/>.
/// </summary>
/// <returns>A future <see cref="ParameterSet"/> and its id. Null if unavailable or cancelled.</returns>
Task<ParameterSetWithId> Propose();
Task<ParameterSetWithId> ProposeAsync();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 This is the only public API change in this pull request. It's not clear to me if/when this API shipped or whether it's allowed to be renamed at this point. If we can't rename it, I'll revert the change to this file and use a suppression instead.

Copy link
Member

Choose a reason for hiding this comment

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

Microsoft.ML.Sweeper hasn't shipped in any nuget package, so I think we are safe to rename this.

@sharwell sharwell requested a review from harishsk February 14, 2020 14:32
@sharwell sharwell marked this pull request as ready for review February 14, 2020 14:32
@sharwell sharwell requested a review from a team as a code owner February 14, 2020 14:32
@sharwell
Copy link
Contributor Author

@harishsk This is now ready for review. Please make specific note of the indicated public API change.

@harishsk
Copy link
Contributor

harishsk commented Mar 2, 2020

The changes have been approved. Can you please rebase and merge?

This rule remains disabled in test code to avoid requiring test methods
to be renamed.
@sharwell sharwell merged commit d26b896 into dotnet:master Mar 2, 2020
@sharwell sharwell deleted the async-suffix branch March 2, 2020 20:18
@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants