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

Audit async/await #851

Closed
crutkas opened this issue Jul 20, 2017 · 11 comments
Closed

Audit async/await #851

crutkas opened this issue Jul 20, 2017 · 11 comments
Assignees
Labels
Can Close Out Soon Work relating to this issue has been completed.
Milestone

Comments

@crutkas
Copy link
Member

crutkas commented Jul 20, 2017

After #844, i think we should do a full audit.

@crutkas crutkas added this to the 1.3 milestone Jul 20, 2017
@crutkas
Copy link
Member Author

crutkas commented Jul 21, 2017

this is what i found inside the Big.sln, few async voids too

code\src\UI\VisualStudio\SolutionWizard.cs -> RunFinished
RunFinished also is async void
SolutionWizard has a lot of non-referenced methods

code\src\UI\ViewModels\NewProject\MainViewModel.cs(166)
async void OnTemplatesAvailable()
async void OnNewTemplatesAvailable()
Both are Async void

code\src\UI\ViewModels\Common\BaseMainViewModel.cs(283)
async void OnCheckUpdates()

code\src\Installer.2017\Commands\RelayCommandPackage.cs(66)
async Task PrepareBootstrapSvc()
async Task CreateService(IAsyncServiceContainer container, CancellationToken cancellationToken, Type serviceType)

code\src\Installer.2017\Commands\GenContextBootstrapService.cs(42)
public async System.Threading.Tasks.Task GenContextInit()

code\test\Templates.Test\StyleCopProjectGenerationTests.cs(69)
async void GenerateAllPagesAndFeaturesAndCheckWithStyleCop(string projectType, string framework)

code\test\Templates.Test\ProjectGenerationTests.cs(150)
async void GenerateAllPagesAndFeaturesRandomNames(string projectType, string framework)
async void GenerateAllPagesAndFeatures(string projectType, string framework)
async void GenerateProjectWithIsolatedItems(string itemName, string projectType, string framework, string itemId)
async void GenerateEmptyProject(string projectType, string framework)
async void GenerateEmptyProject(string projectType, string framework)

code\src\Core\Locations\TemplatesSynchronization.cs(206)
async Task CheckContentUnderVersion()
async Task CheckContentOverVersion()
async Task CheckInstallDeployedContent()
async Task Do()

code\src\Core\Diagnostics\TelemetryTracker.cs(146)
async Task TrackEditSummaryItem(EditItemActionEnum trackedAction)

code\test\VsEmulator\Main\MainViewModel.cs(179)
async void NewProject()

test\Core.Test\TestData\Merge\Source_expected.cs(93):
async void OnLaunched(LaunchActivatedEventArgs e)

code\test\Core.Test\TestData\Merge\Source.cs(88)
async void OnLaunched(LaunchActivatedEventArgs e)

@clairernovotny
Copy link
Member

You don't need to do this by hand.... Use this: https://www.nuget.org/packages/Microsoft.VisualStudio.Threading.Analyzers

Calls these out in the IDE. You can also configure them as either warnings or errors if you want to hard-fail.

@crutkas
Copy link
Member Author

crutkas commented Aug 18, 2017

Zomg. I need to chat with you more often!

@crutkas
Copy link
Member Author

crutkas commented Aug 18, 2017

@mrlacey how does the analyzers work for the stylecop checks. we should add this in as well there.

@mrlacey mrlacey self-assigned this Aug 18, 2017
@mrlacey
Copy link
Collaborator

mrlacey commented Aug 18, 2017

@crutkas Can add that package to the Secret StyleCop template, set warnings as errors and it would be caught as part of the test....i think

mrlacey added a commit that referenced this issue Aug 18, 2017
@mrlacey mrlacey removed their assignment Aug 18, 2017
@mrlacey
Copy link
Collaborator

mrlacey commented Aug 18, 2017

I've added the analyzer to the main projects and fixed the naming issues and a few easy other things. It's also highlighted a bunch of other issues that are more involved and will take longer to fix than I have time in the next week so leaving this open.
Need to address or suppress (with good reasons and comments) the warnings raised by the analyzer.

I tried adding the analyzer to the StyleCop tests but as the Analyzer covers a lot of other things it found a lot of false positives that show this isn't the best way to verify the naming in the generated projects. Have created #969 to track that as a separate issue.

mrlacey added a commit that referenced this issue Aug 24, 2017
#851
+ a few other miscellaneous warnings
@crutkas
Copy link
Member Author

crutkas commented Aug 24, 2017

with #1002, this can be closed out, no?

@crutkas
Copy link
Member Author

crutkas commented Aug 24, 2017

ahem, @mrlacey with #1002, this can be closed?

@mrlacey
Copy link
Collaborator

mrlacey commented Aug 24, 2017

This is still raising some warnings, but they're complicated to fix.

It's also highlighted a bunch of other issues that are more involved and will take longer to fix than I have time in the next week so leaving this open.
Need to address or suppress (with good reasons and comments) the warnings raised by the analyzer.

Could close this and raise another issue to follow up but had left this open and unassigned to address this here. Could close on the basis that the audit has been done but this feels just like moving the work along.

@crutkas
Copy link
Member Author

crutkas commented Aug 24, 2017

lets close, log bugs against, and then suppress them

mrlacey added a commit that referenced this issue Aug 24, 2017
audit was for #851
follow up task #868 already planned
@mrlacey mrlacey self-assigned this Aug 24, 2017
@mrlacey mrlacey added the Can Close Out Soon Work relating to this issue has been completed. label Aug 24, 2017
@mrlacey
Copy link
Collaborator

mrlacey commented Aug 29, 2017

don't see a reason to keep this open

@mrlacey mrlacey closed this as completed Aug 29, 2017
@ghost ghost locked as resolved and limited conversation to collaborators Jun 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Can Close Out Soon Work relating to this issue has been completed.
Projects
None yet
Development

No branches or pull requests

3 participants