-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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] Make WasmMainJSPath
optional
#81484
Conversation
Tagging subscribers to 'arch-wasm': @lewing Issue Details
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
# Conflicts: # src/mono/sample/wasi/Directory.Build.targets # src/mono/sample/wasm/Directory.Build.targets # src/mono/sample/wasm/browser/Wasm.Browser.Sample.csproj # src/mono/sample/wasm/console-v8/Wasm.Console.V8.Sample.csproj
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-wasm-libtests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
Failure is unrelated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments but LGTM. It would be good to hear from @radical
/azp run runtime |
Pull request contains merge conflicts. |
# Conflicts: # src/tasks/WasmAppBuilder/WasmAppBuilder.cs
@@ -13,6 +12,7 @@ | |||
|
|||
<ItemGroup> | |||
<WasmExtraFilesToDeploy Include="index.html" /> | |||
<WasmExtraFilesToDeploy Include="main.js" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding these in the various places outside the wasm targets can be dropped now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the one comment, LGTM! Good job 👍
@maraf I resolved the merge conflicts. Hopefully, I didn't break anything! |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
WasmMainJSPath
required only for console targets (v8/node).WasmMainJSPath
from browser projects.Closes #63465