Skip to content

ci: Add Swift version matrix for 6.0, 6.2, and 6.3 testing#10

Open
doozMen wants to merge 2 commits intofix/update-readme-limitationsfrom
fix/ci-swift-version-matrix
Open

ci: Add Swift version matrix for 6.0, 6.2, and 6.3 testing#10
doozMen wants to merge 2 commits intofix/update-readme-limitationsfrom
fix/ci-swift-version-matrix

Conversation

@doozMen
Copy link
Owner

@doozMen doozMen commented Dec 13, 2025

Update the CI workflow to test across multiple Swift versions:

  • Xcode 16.0 (Swift 6.0) - debug and release
  • Xcode 16.2 (Swift 6.2) - debug and release
  • Xcode 16.4 (Swift 6.3) - debug and release

This ensures compatibility is maintained across Swift versions as
the library evolves.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com

Copy link
Owner Author

doozMen commented Dec 13, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link

claude bot commented Dec 13, 2025

Code Review: Swift Version Matrix CI Update

Summary

This PR enhances the CI workflow by testing across multiple Swift versions (6.0, 6.2, 6.3), improving compatibility verification. The change is well-structured and addresses an important testing gap.

✅ Strengths

  1. Excellent test coverage expansion - Testing across three Swift versions (6.0, 6.2, 6.3) ensures compatibility as noted in the README's Swift 6.3 compatibility section
  2. Good matrix structure - Using include rather than Cartesian product gives explicit control over combinations
  3. Clear job naming - The updated job name provides excellent visibility in CI runs with Swift version and config displayed
  4. Both debug and release builds - Testing both configurations for each Swift version is thorough and best practice

🔍 Observations & Considerations

1. Xcode/Swift Version Mapping (Medium Priority)

The PR assumes this mapping:

  • Xcode 16.0 → Swift 6.0
  • Xcode 16.2 → Swift 6.2
  • Xcode 16.4 → Swift 6.3

Verification needed: These mappings should be verified as Xcode versions don't always have a 1:1 correspondence with Swift versions. For example:

  • Xcode 16.0 typically ships with Swift 6.0 ✓
  • Xcode 16.2 might ship with Swift 6.1 or 6.2 (needs verification)
  • Xcode 16.4 is future/beta - ensure it's available on macos-15 runners

Recommendation: Add a step to verify Swift version after Xcode selection:

- name: Verify Swift version
  run: swift --version

2. CI Runtime Duration (Low Priority)

The matrix now runs 6 jobs instead of 2 (3x Swift versions × 2 configs). This will:

  • Increase CI time by ~3x (jobs may run in parallel depending on runner availability)
  • Increase GitHub Actions minutes usage by ~3x

For a library focused on compatibility, this trade-off is worthwhile. However, consider:

  • Whether all PRs need all 6 jobs, or if some could be on: push to main only
  • Using fail-fast: false to see all failures rather than stopping at first failure:
strategy:
  fail-fast: false
  matrix:
    # ...

3. Alignment with README (Low Priority)

README.md:24-37 documents Swift 6.3 compatibility issues with CloudKit. This PR ensures those issues are caught in CI, which is great! However:

  • Consider if Swift 6.0 and 6.2 should also explicitly test CloudKit functionality
  • The tests run via swift test -c <config> - confirm these exercise CloudKit paths or if they're conditionally disabled

4. Examples Job Inconsistency (Low Priority)

The examples job at .github/workflows/ci.yml:44 still only tests Xcode 16.4. Consider whether examples should also test across multiple Swift versions for consistency.

🐛 Potential Issues

None identified - The YAML syntax is correct, the matrix structure is valid, and the job configuration looks sound.

🔒 Security Concerns

None - This is a CI configuration change with no security implications.

🎯 Performance Considerations

The change itself has no runtime performance impact on the library. CI performance is discussed above.

✅ Test Coverage

The PR appropriately expands test coverage across Swift versions. The existing test suite (swift test) will now run against 3 Swift versions in 2 configurations each.

📝 Recommendations Summary

  1. Add Swift version verification step to confirm Xcode/Swift mapping
  2. Consider adding fail-fast: false to see all failures across versions
  3. Verify Xcode 16.2 and 16.4 availability on macos-15 runners
  4. Optional: Document expected CI runtime in PR description for future reference

Verdict

Approve with minor suggestions - This is a solid improvement to CI that aligns well with the project's Swift 6.3 compatibility goals. The suggestions above are enhancements rather than blockers.


🤖 Review generated by Claude Code

Stijn Willems and others added 2 commits December 13, 2025 22:07
Update the CI workflow to test across multiple Swift versions:
- Xcode 16.0 (Swift 6.0) - debug and release
- Xcode 16.2 (Swift 6.2) - debug and release
- Xcode 16.4 (Swift 6.3) - debug and release

This ensures compatibility is maintained across Swift versions as
the library evolves.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Uses the branch that guards PerceptionCore.Bindable usage for
Swift 6.0+ compatibility with swiftbuild.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@doozMen doozMen force-pushed the fix/ci-swift-version-matrix branch from 57a9001 to f1ebe42 Compare December 13, 2025 21:39
@claude
Copy link

claude bot commented Dec 13, 2025

PR Review: CI Swift Version Matrix

Summary

This PR adds comprehensive Swift version testing to the CI workflow, testing across Swift 6.0, 6.2, and 6.3 with both debug and release configurations. Overall, this is a valuable improvement to the testing infrastructure.


✅ Strengths

1. Comprehensive Version Coverage

  • Tests across three Swift versions (6.0, 6.2, 6.3) provide good compatibility coverage
  • Both debug and release configurations ensure build configuration parity
  • Addresses the Swift 6.3 compatibility concerns mentioned in README.md:24-37

2. Clear Job Naming

  • The job name macOS (Swift ${{ matrix.swift }}, ${{ matrix.config }}) provides excellent visibility in the CI dashboard
  • Makes it easy to identify which specific configuration failed

3. Proper Matrix Structure

  • Uses include pattern for explicit version mapping, which is more maintainable than combinatorial matrices
  • Correctly maps Xcode versions to Swift versions (16.0→6.0, 16.2→6.2, 16.4→6.3)

⚠️ Issues & Concerns

1. Critical: Unstable Dependency (Severity: High)
.github/workflows/ci.yml:35 and Package.swift:35

The PR switches from a pinned revision to a branch reference:

- .package(url: "https://github.com/doozMen/swift-sharing", revision: "f1170dc9b28faea3edec20705839eb2bc349bcdb"),
+ .package(url: "https://github.com/doozMen/swift-sharing", branch: "fix/swift-build-compatibility"),

Problems:

  • Branch dependencies are non-deterministic and can break builds without warning
  • The branch could be force-pushed, deleted, or rebased
  • Violates Swift Package Manager best practices for production dependencies
  • The TODO comment indicates this is temporary, but it shouldn't be merged in this state

Recommendation:

  • Wait for the swift-sharing PR to be merged
  • Pin to a specific commit or tag before merging this PR
  • If urgency requires, at minimum update the TODO with a clear timeline and tracking issue

2. Package.resolved Changes (Severity: Medium)
Package.resolved:1-133

The package resolution changes show several dependencies moving from branch references to specific revisions:

  • GRDB.swift: removed branch: master
  • swift-structured-queries: removed branch: main

However, swift-sharing moves in the opposite direction (revision → branch). This inconsistency suggests the dependency management strategy needs clarification.

3. Missing Examples Matrix Update (Severity: Low)
.github/workflows/ci.yml:46-50

The examples job still only tests with Xcode 16.4. Consider whether examples should also be tested across multiple Swift versions, especially since the README mentions Swift 6.3 compatibility issues with CloudKit.

4. CI Build Time Impact (Severity: Low)

The change increases CI matrix from 2 jobs to 6 jobs (3x increase). This is acceptable for comprehensive testing, but be aware of:

  • Increased GitHub Actions minutes consumption
  • Longer feedback cycles for PR checks
  • Potential for more flaky test failures

🔍 Security & Performance

Security: ✅ No security concerns identified

Performance:

  • CI runtime will increase proportionally (2 jobs → 6 jobs)
  • Each job remains independent, so parallelization helps
  • Consider adding fail-fast: false to the matrix strategy to see all version failures, not just the first

📋 Testing

Test Coverage: ⚠️ Partially adequate

  • The workflow tests library code across versions
  • Examples are only tested on Swift 6.3 (Xcode 16.4)
  • No explicit validation that the dependency change fixes Swift 6.0 compatibility

Recommendation:
Before merging, verify that the new swift-sharing branch actually resolves the Swift 6.0 build issues mentioned in the PR description.


🎯 Best Practices

Code Quality: ✅ Good

  • Clean YAML structure
  • Proper use of matrix includes
  • Clear variable naming

Git Hygiene: ✅ Good

  • Focused commits with clear messages
  • Appropriate co-authorship attribution

📝 Suggestions

  1. Add fail-fast control:
strategy:
  fail-fast: false
  matrix:
    include:
      # ... existing matrix
  1. Document minimum supported Swift version:
    Update README.md or Package.swift to explicitly state the minimum Swift version (appears to be 6.0)

  2. Consider adding a comment explaining the Xcode→Swift mapping:

# Xcode 16.0 = Swift 6.0
# Xcode 16.2 = Swift 6.2  
# Xcode 16.4 = Swift 6.3

🚫 Blocking Issues

Must be resolved before merge:

  1. ❌ Replace branch dependency with pinned revision/tag in Package.swift:35
    • The TODO comment acknowledges this is temporary
    • This creates build instability and should not be merged

Final Recommendation

REQUEST CHANGES - Do not merge until the swift-sharing dependency is pinned to a stable reference (commit SHA or tag). The CI matrix improvements are excellent, but the unstable dependency introduces too much risk.

Once the dependency is properly pinned, this will be a valuable addition to the project's quality assurance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant