-
-
Notifications
You must be signed in to change notification settings - Fork 50
Add Wasm and Android platforms #3803
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
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
5401652
to
6d47143
Compare
The builder capable of building for wasm and android is live. This PR is ready to be merged, we just need to decide to what extent we want to update the matrix before deploying to prod. |
6d47143
to
848a5b8
Compare
👍 Is there a CSS way to rotate the headers by 30 or 45 degrees? It might be all that's needed tbh. |
Just for the purposes of merging, I'd suggest we merge those changes separately BTW. |
If you'd like to do that then let's remove the cells from this one so we don't merge it looking like this. However, I should have this done today if we can hold off until then as I'm right in the middle of it and having to undo those cell changes and put them back in on a different branch will just delay me. |
I don't think removing them is worth the effort. I can ad hoc deploy from the branch to keep dev on this version. But when the other PR is ready we should merge them sequentially without tagging in between and then they're still separate PRs for the purpose of referencing them. (I do keep references to similar previous tasks in places as a template for future tasks and it'd be better if it didn't contain extra UI changes.) |
That's fine, but let's just HTML comment out the extra cells then. They can even stay in the Full Build Details page, but I don't want to deploy the matrix like this. |
@coderabbitai please review |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 3917 🏁 Script executed:
Length of output: 28333 🏁 Script executed:
Length of output: 55449 @daveverwer I've reviewed PR #3803 which adds Wasm and Android platform support to the Swift Package Index. The implementation looks solid and well-structured:
I can see from the PR comments that there are concerns about the matrix UI becoming crowded with the additional platforms. The screenshots show that the headers might be difficult to read. The proposed CSS transform ( Is there anything specific you'd like me to examine more closely about the implementation? |
@daveverwer @finestructure Any thoughts on adding an additional badge for What I need is for the build checker to run The 0.0 version of WASI that I'm not sure |
Thanks for your comment, @scottmarchant . Would you like to open a discussion about this? We can't afford to run multiple Wasm variants at this time but it would be good to understand if there's a better SDK/triple we could choose for the builds that we do run. |
@finestructure I learned that |
Ah, good to know, thanks for the update! |
To do
BUILD_TRIGGER_DOWNSCALING
to 0.01We need to deploy this to dev in order to finalise the builder changes, because they need to report back to the API in order to pass the tests.
We can ad hoc deploy from this branch without merging to allow the builder tests to pass.
I've set the
BUILD_TRIGGER_DOWNSCALING
for dev to 0 in order to avoid triggering any of the new platform builds while it is running on dev.