updates from AtLeast#11
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v1.0.0-alpha.3 #11 +/- ##
===============================================
Coverage 74.46% 74.46%
===============================================
Files 47 47
Lines 1907 1907
===============================================
Hits 1420 1420
Misses 487 487
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PR Review: updates from AtLeastOverall, this PR makes solid CI infrastructure improvements. A few issues worth discussing before merge. Bugs / Inconsistencies
In # ensure-remote-deps.sh
-REMOTE_BRANCH="48-demo-applications-part-3"
+REMOTE_BRANCH="bitness-v0.0.8" # ← but main CI uses atleast-beta.6Either Code Quality
// Package.swift
.package(name: "SundialKit", path: "../SundialKit")A contributor who clones only this repo gets a build failure immediately. The comment says "CI rewrites this to a remote URL" but says nothing about local development. This should either:
This is a consequence of the CI rewriting Package.swift to a 40-char SHA URL. The workaround is pragmatic but it's unusual to carry a lint suppression comment in a package manifest for a purely CI-generated line. Worth noting as technical debt if a cleaner mechanism is available. Structural / Merge Concerns
After merging to
If this is intended as temporary scaffolding for ongoing co-development work, it should be tracked for cleanup. If it's permanent (main always builds against this SundialKit branch), that coupling should be documented. Positive Changes
|
…ibility The ensure-remote-deps.sh script expects the dependency format to include the name parameter. This change ensures the CI pipeline can properly convert the local path to the remote URL. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add skip-package-resolved: true to both Ubuntu and macOS build jobs to allow dynamic dependency resolution instead of using Package.resolved pins. This prevents version compatibility issues during CI builds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Subrepos pulled at atleast-beta.6 (post-merge tips 8e5f0cd / 9c3d62e — main merged in upstream so the modernized CI baseline is now there). Local AtLeast-side wiring: - AtLeastKit/Package.swift switched to .package(path:) for both libs - atleast.yml: broaden release matrix to [6.2, 6.3], add nightly container expression (forward-compatible for 6.4 nightly), bump simulator osVersion 26.4 → 26.5 In-subrepo refresh (partial — mise pins only): - SundialKit + SundialKitStream .mise.toml → MistKit v1.0.0-beta.3 pins (aqua SwiftLint 0.62.2, periphery 3.7.4) Resume notes in .claude/docs/SUNDIALKIT_CI_REFRESH_RESUME.md cover: - Remaining Phase B edits (workflow + codeql.yml Xcode 26.5 bumps, checkout v4→v6, macos-15→macos-26 — different codeql-action handling for the two libs) - Verification steps - git subrepo push back to atleast-beta.6 - Hand-off to SUNDIALKIT_TRANSFERUSERINFO.md Phase 1-3 ci skip Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hain Phase B remainder of the vendor + CI refresh: - SundialKit.yml / SundialKitStream.yml: Xcode 26.4 -> 26.5, simulator osVersion 26.4 -> 26.5 (back-compat Xcode 16.4 / macOS-15 rows left intact) - sundial-demo.yml: same Xcode/osVersion bumps for the TestFlight demo jobs - codeql.yml (both): checkout v4 -> v6, macos-15 -> macos-26, Xcode 16.4 -> 26.5 - SundialKit codeql.yml: drop merged v2.0.0 PR branch, bump codeql-action v3 -> v4 (SundialKitStream already on v4) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Enable CI on the long-lived beta branches that were previously untriggered: - AtLeast atleast.yml: add iphone-app-beta.6 to push branches; treat it like main in the configure job (FULL + CROSS_PLATFORM) so Windows/Android/all Apple platforms build. - SundialKit / SundialKitStream: add atleast-beta.6 to push + pull_request branches and to the full-matrix condition. PRs already triggered AtLeast CI; the gap was push to these feature branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ne CI The standalone brightdigit/SundialKitStream CI checks out only itself, so the local `path: "../SundialKit"` dependency cannot be found and every build job fails. Adopt the brightdigit setup-<kit> pattern (mirrors MistKit): add a setup-sundialkit composite action that, in CI only, rewrites Package.swift to a remote url dep pinned to the branch's current commit and drops Package.resolved. Wire it into all five build jobs after checkout, passing skip-package-resolved to swift-build. The source Package.swift keeps the path dependency so monorepo local development is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Swift 6.x Android SDK sysroot ships CRT startup objects only up to API 35; targeting api 36 fails at link with "cannot open crtbegin_dynamic.o / crtend_android.o". 33/34/35 link fine. Cap matrices at 35 until the SDK bundles an NDK r28+ sysroot: - atleast.yml: [33,34,35,36] -> [33,34,35] - SundialKit: [33,34,35,36] -> [33,34,35] - SundialKitStream: [33,36] -> [33,35] Tracking: skiptools/swift-android-action#27 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The setup-sundialkit CI action rewrites the local path dependency to a remote URL pinned by a 40-char revision, producing a 120-char line that exceeds the 108-char limit. Disable line_length on that line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Code Review: updates from AtLeast (PR 11) Overview This PR updates the CI infrastructure to adopt a new composite action for managing the SundialKit dependency, switching Package.swift from a versioned remote dependency to a local path dependency, and removing the old ensure-remote-deps.sh script. Tool versions also bumped: Xcode 26.5, SwiftLint 0.62.2, periphery 3.7.4, CodeQL v4. What Works Well
Issues and Suggestions
The comment says CI rewrites this to a remote URL pinned by 40-char revision, but the CI changes show a local checkout approach via setup-sundialkit (checking out SundialKit at ../SundialKit). If the composite action still performs a URL rewrite internally, make that explicit. If CI now only uses the local checkout, the swiftlint:disable:next line_length directive is suppressing a warning for a line that does not violate the limit. Suggestion: Clarify the comment to match actual CI behavior.
With Package.swift pointing to path: ../SundialKit, anyone cloning only this repository gets a build error (local dependency not found). The old approach with a remote URL and semver range worked out of the box. This is a DX regression for contributors not also working on SundialKit. Suggestion: Document the sibling-directory requirement in README.md or CLAUDE.md.
Transitive dependency versions are no longer source-controlled. Acceptable with a local path dependency, but worth restoring once a released SundialKit version is targeted.
atleast-beta.6 is hardcoded in workflow trigger branches, the SUNDIALKIT_BRANCH env var, and the composite action ref. These must be updated before merging into the release line.
Two consecutive blank lines appear after the Setup SundialKit step before the Initialize CodeQL comment.
A 5-minor-version jump within v3. Newer periphery may flag previously-undetected dead code causing CI lint failures. Suggest running LINT_MODE=STRICT ./Scripts/lint.sh locally before merging.
Switching from swiftlint 0.61.0 to aqua:realm/SwiftLint 0.62.2 changes both the version and the installation backend. Verify the aqua provider works in all CI environments (macOS, Ubuntu, Windows). Summary
Overall this is a solid infrastructure modernisation. The primary concern is the local-path dependency making standalone development harder, and the inaccurate comment in Package.swift. Both are straightforward to address before merging. |
Point both the build dependency (SUNDIALKIT_BRANCH) and the setup-sundialkit action pin at SundialKit's v2.0.0-alpha.3 ref. The SundialKitStream-branch triggers are left unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
No description provided.