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

[wasm] Add warning code to all warnings produced in WasmAppBuilder #78755

Merged
merged 3 commits into from
Dec 2, 2022

Conversation

maraf
Copy link
Member

@maraf maraf commented Nov 23, 2022

  • WASM0001 - All PInvoke related warnings
  • WASM0002 - Missing CultureName metadata for satellite assembly
  • WASM0003 - Found identical vfs mappings for target path
  • WASM0004 - Loading assembly reference '..' for '..' failed

Fixes #78690

@maraf maraf added arch-wasm WebAssembly architecture area-Build-mono labels Nov 23, 2022
@maraf maraf added this to the 8.0.0 milestone Nov 23, 2022
@maraf maraf self-assigned this Nov 23, 2022
@maraf maraf requested a review from radical as a code owner November 23, 2022 09:32
@ghost
Copy link

ghost commented Nov 23, 2022

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

Issue Details
  • WASM0001 - All PInvoke related warnings
  • WASM0002 - Missing CultureName metadata for satellite assembly
  • WASM0003 - Found identical vfs mappings for target path
  • WASM0004 - Loading assembly reference '..' for '..' failed

Fixes #78690

Author: maraf
Assignees: maraf
Labels:

arch-wasm, area-Build-mono

Milestone: 8.0.0

@maraf
Copy link
Member Author

maraf commented Nov 23, 2022

@radical Should I add warning code even for this one?

Log.LogWarning(reason);

@maraf maraf changed the title [wasm] Add warning code to all warning produced in WasmAppBuilder [wasm] Add warning code to all warnings produced in WasmAppBuilder Nov 23, 2022
@radical
Copy link
Member

radical commented Nov 23, 2022

@radical Should I add warning code even for this one?

Log.LogWarning(reason);

Good question. Remove the LogWarning. The caller will log the reason as a MessageImportance.Low message.
And change the message to the dependency file {inFile} needed for compiling {srcFile} to {outFile} could not be found., so it works better with the full message that is logged by the caller.

@radical
Copy link
Member

radical commented Nov 23, 2022

@akoeplinger @steveisok do we need to document these new warning codes anywhere?

@akoeplinger
Copy link
Member

do we need to document these new warning codes anywhere?

Yes but I think we also need to localize them if they're actually user-facing. I don't know if there's guidance/restrictions around coming up with new codes though.

@radical
Copy link
Member

radical commented Dec 2, 2022

I have opened #79173 for following up on the localization, and documentation.

@radical
Copy link
Member

radical commented Dec 2, 2022

The MacCatalyst failure is unrelated, and was - #78778 .

@radical radical merged commit bcc818f into dotnet:main Dec 2, 2022
@maraf maraf deleted the WasmNativeWarning branch December 5, 2022 08:33
@maraf
Copy link
Member Author

maraf commented Dec 5, 2022

/backport to release/7.0

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3622382639

@ghost ghost locked as resolved and limited conversation to collaborators Jan 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebAssembly.Sdk - Unsuppressable warnings
3 participants