-
Notifications
You must be signed in to change notification settings - Fork 0
feat: macOS ICU build #4
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
Conversation
c4da891 to
89d2248
Compare
3858713 to
f81fd1c
Compare
51effc1 to
fb22ed9
Compare
7fc1fb9 to
9763112
Compare
42055b0 to
050264b
Compare
fab8f7d to
193c373
Compare
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.
Pull Request Overview
This PR adds macOS support to the Uno ICU library by creating a new uno.icu-macos NuGet package and updating the build process to compile ICU libraries for macOS platforms.
- Adds build jobs for creating universal macOS ICU binaries (x86_64 and arm64)
- Creates new
uno.icu-macosNuGet package specification for macOS ICU libraries - Restructures the existing build process to support multiple target platforms
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/main.yml |
Adds macOS build jobs and restructures artifact handling to support both WASM and macOS targets |
nuget/uno.icu-macos/uno.icu-macos.nuspec |
Creates NuGet package specification for macOS ICU libraries |
nuget/uno.icu-wasm/buildTransitive/Uno.icu-wasm.targets |
Adds MSBuild targets for WASM native file references |
nuget/uno.icu-wasm/build/Uno.icu-wasm.targets |
Removes WebAssembly-specific condition from embedded resource inclusion |
nuget/buildTransitive/Uno.icu-wasm.targets |
Removes old build targets file (moved to package-specific location) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
jeromelaban
left a comment
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.
aside from copilot's comments, looks good
spouliot
left a comment
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.
The changes LGTM but I lack context. Is this only for web assembly on macOS ?
|
@spouliot No, it's for the desktop target. I'm working on a new text handling implementation that is Unicode-aware, which uses ICU for stuff like line segmentation and BiDi, so we need a package that provides the ICU binaries. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
GitHub Issue (If applicable): #
PR Type
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior?
PR Checklist
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.Other information
Internal Issue (If applicable):