try android CI with .when and add some relationship required basics#23
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds ULID-based model retrieval by introducing a public ChangesCore Features and Platform Support
Android CI Infrastructure
Possibly related PRs
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext+Relationship.swift (1)
4-10: ⚡ Quick winDatabase fetch pattern does not optimize for single-record lookups.
The current implementation uses
fetchAll().firstwhich executes the full query and returns an array before selecting the first element. While theidpredicate ensures only one match, the underlying_fetchAlldoesn't apply a LIMIT 1 optimization at the SQL level. No specialized single-record fetch method currently exists in the codebase (nofetchOne,fetchFirst, or similar). If this becomes a performance concern, consider implementing a dedicated single-record fetch method.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext`+Relationship.swift around lines 4 - 10, The getModel method currently calls fetchAll(...).first which materializes a full result set; add a dedicated single-record fetch to avoid that: implement a new fetchOne/fetchFirst function (e.g., fetchOne<T: PersistentModel>(_: PredicateBuilder<T>)) that reuses the underlying query execution path but sets a LIMIT 1 (or uses the lower-level _fetchAll/_executeQuery with a limit flag), and change ManagedObjectContext.getModel(id:type:) to call fetchOne(PredicateBuilder<T>().addCheck(.isEqualTo, "id", id)) instead of fetchAll(...).first; ensure the new fetchOne returns an optional T? and bubbles MOCError the same way as fetchAll so callers and identityMap logic remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/swift-test-android.yml:
- Around line 12-14: The workflow uses mutable tags for actions and leaves
checkout credentials persisted; replace the tag references actions/checkout@v6
and skiptools/swift-android-action@v2 with their corresponding immutable commit
SHAs (pin to specific commit SHAs) and update the checkout step to disable
credential persistence by adding persist-credentials: false to the
actions/checkout invocation; ensure the pinned SHA strings are used exactly
where actions/checkout and skiptools/swift-android-action are referenced so the
workflow runs a fixed, auditable revision.
---
Nitpick comments:
In
`@Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext`+Relationship.swift:
- Around line 4-10: The getModel method currently calls fetchAll(...).first
which materializes a full result set; add a dedicated single-record fetch to
avoid that: implement a new fetchOne/fetchFirst function (e.g., fetchOne<T:
PersistentModel>(_: PredicateBuilder<T>)) that reuses the underlying query
execution path but sets a LIMIT 1 (or uses the lower-level
_fetchAll/_executeQuery with a limit flag), and change
ManagedObjectContext.getModel(id:type:) to call
fetchOne(PredicateBuilder<T>().addCheck(.isEqualTo, "id", id)) instead of
fetchAll(...).first; ensure the new fetchOne returns an optional T? and bubbles
MOCError the same way as fetchAll so callers and identityMap logic remain
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2180f67-035b-46e2-80cb-30b914d0ce1f
📒 Files selected for processing (4)
.github/workflows/swift-test-android.ymlPackage.swiftSources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext+Relationship.swiftSources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext.swift
…into feat/relationship # Conflicts: # .github/workflows/swift-test-linux.yml
Summary
This PR introduces Android CI support and refactors dependency management with conditional platform targeting, along with new relationship model lookup capabilities.
Changes
GitHub Actions Workflow
.github/workflows/swift-test-android.ymlto test Swift packages on Android viaskiptools/swift-android-action@v2free-disk-space: trueto mitigate emulator installation failures on space-constrained runnersDependency Management Improvements
Package.swiftto use.product(...)entries withcondition: .when(platforms: ...)for cleaner conditional dependency declarationULID,KeychainAccess(Apple platforms: iOS, macOS, tvOS, visionOS, watchOS), andKeyringAccess(Linux) dependencies within the main arrayRelationship Model Support
ManagedObjectContext.getModel(id:type:)method to retrieve persisted models byULIDMOCErrorfor error handlingPlatform-Specific Key Retrieval
ManagedObjectContext.getKeyForDatabase()with explicit Linux platform branch usingKeyringAccessnilHighlights
The adoption of Swift's
.when()conditional syntax for dependency declaration is a significant improvement—it moves from imperative conditional logic to declarative platform targeting, making the Package.swift manifest more maintainable and aligned with modern Swift package conventions. This pattern is particularly elegant given the complexity of supporting multiple Apple platforms and Linux.