Skip to content

[wasm] Fix analyzer support in templates - #77704

Merged
radical merged 3 commits into
dotnet:mainfrom
radical:fix-sample-analyzer
Nov 1, 2022
Merged

[wasm] Fix analyzer support in templates#77704
radical merged 3 commits into
dotnet:mainfrom
radical:fix-sample-analyzer

Conversation

@radical

@radicalradical commented Nov 1, 2022

Copy link
Copy Markdown
Member

@radicalradical added arch-wasm WebAssembly architecture area-Build-mono labels Nov 1, 2022
@ghost

ghost commented Nov 1, 2022

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details
Author:radical
Assignees:-
Labels:

arch-wasm, area-Build-mono

Milestone:-

@ghostghost assigned radicalNov 1, 2022
@radical

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@radical

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

- Add
`[assembly:System.Runtime.Versioning.SupportedOSPlatform("browser")]`
to the browser, and console templates. This would allow the analyzers,
if enabled, to treat the assembly as one that will run only on
browser.
- Also, populate `@(SupportedPlatform)` with only `browser`, for *wasm*
projects, similar to https://github.com/dotnet/sdk/blob/fef8cedfb6b4ac85a7e135f3e4f155e29cdcbdf1/src/BlazorWasmSdk/Targets/Microsoft.NET.Sdk.BlazorWebAssembly.5_0.targets#L75-L79
@radical

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@radical

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

[assembly:System.Runtime.Versioning.SupportedOSPlatform("browser")]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this the best approach we can offer?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

IIUC, this is needed. I'm not sure how else we can do this, but open to suggestions!

@pavelsavara

Copy link
Copy Markdown
Member

If this is the right way how to do it, then the samples\wasm should get the same treatment and then we could <RunAnalyzers>true</RunAnalyzers> in it.

@radical

Copy link
Copy Markdown
MemberAuthor

@akoeplinger does my implementation look correct?

I added it for the samples, but didn't enable the analyzers. We could try that.

@pavelsavara

Copy link
Copy Markdown
Member

Related #77712 and #77630

@radical

Copy link
Copy Markdown
MemberAuthor

This is good to go, once we have an approval.

@radical

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@steveisok
steveisok self-requested a review November 1, 2022 17:03
@radical
radical merged commit 6631b47 into dotnet:mainNov 1, 2022
@radical
radical deleted the fix-sample-analyzer branch November 1, 2022 17:48
@ghostghost locked as resolved and limited conversation to collaborators Dec 1, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@radical@pavelsavara@steveisok@maraf