Repository navigation
Follow-up: build only universal nightly artifacts - #1332
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThe workflow consolidates Apple Silicon (arm64) and universal build steps into a single universal nightly build, removing arm-specific artifact paths, verification checks, and appcast variants. Updated references throughout to use the universal app directory and removes arm64-specific bundle ID messaging. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa96222aa0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -425,7 +393,6 @@ jobs: | |||
| exit 1 | |||
| fi | |||
| ./scripts/sparkle_generate_appcast.sh "$NIGHTLY_DMG_IMMUTABLE" nightly appcast.xml | |||
There was a problem hiding this comment.
This step now generates only appcast.xml, but existing installs from the prior universal track were configured with SUFeedURL pointing to appcast-universal.xml (set in this same workflow before this commit). Once this change lands, those clients will stop receiving nightly updates because their feed is no longer regenerated or published, so users already on com.cmuxterm.app.nightly.universal are effectively stranded unless they manually reinstall from a different track.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/nightly.yml (1)
157-164:⚠️ Potential issue | 🟠 MajorUpdate nightly workflow tests to match universal-only behavior
This consolidation breaks current assertions in
tests/test_nightly_universal_build.shthat still require dual Apple Silicon + universal lanes, arm-only lipo checks, and universal-suffixed artifact/appcast names. CI will fail until those tests are updated to the new single-track contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/nightly.yml around lines 157 - 164, The nightly workflow now builds a single universal binary via the xcodebuild invocation used in the "Build universal nightly app (Release)" step, which invalidates assertions in tests/test_nightly_universal_build.sh that expect dual Apple Silicon + universal lanes, arm-only lipo checks, and artifact/appcast names with architecture-specific suffixes; update tests/test_nightly_universal_build.sh to (1) remove/replace checks that assert both arm64 and universal lanes and any branch logic for separate arm-only lipo validation, (2) change lipo/arch assertions to verify the produced binary is universal (contains arm64 and x86_64) instead of expecting an arm-only archive, and (3) normalize artifact/appcast name expectations to the new universal-only naming (remove architecture-specific suffixes like -arm64 or -universal variants) so the test reflects the single-track contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/nightly.yml:
- Around line 157-164: The nightly workflow now builds a single universal binary
via the xcodebuild invocation used in the "Build universal nightly app
(Release)" step, which invalidates assertions in
tests/test_nightly_universal_build.sh that expect dual Apple Silicon + universal
lanes, arm-only lipo checks, and artifact/appcast names with
architecture-specific suffixes; update tests/test_nightly_universal_build.sh to
(1) remove/replace checks that assert both arm64 and universal lanes and any
branch logic for separate arm-only lipo validation, (2) change lipo/arch
assertions to verify the produced binary is universal (contains arm64 and
x86_64) instead of expecting an arm-only archive, and (3) normalize
artifact/appcast name expectations to the new universal-only naming (remove
architecture-specific suffixes like -arm64 or -universal variants) so the test
reflects the single-track contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a422d610-0115-414f-94bb-5f7a20f0a4da
📒 Files selected for processing (1)
.github/workflows/nightly.yml
This branch was successfully deployed
1 participant
Summary
cmux-nightly-macos.dmgandappcast.xmltrack, but make it universalTesting
ruby -e '''require "yaml"; YAML.load_file(".github/workflows/nightly.yml"); puts "YAML OK"'''git diff --checkIssues
Summary by cubic
Build only the universal nightly app and publish it under the existing nightly track to simplify CI and remove duplicate artifacts. Users now get a single universal DMG via
appcast.xml.cmux-nightly-macos.dmgandappcast.xml, now produced from the universal build.cmux-nightly-universal-macos.dmg,appcast-universal.xml).Written for commit aa96222. Summary will update on new commits.
Summary by CodeRabbit