Discussion: concurrency direction — GCD TaskManager vs async/await - #8
Conversation
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds documentation for choosing Swift Concurrency, planning the migration through stacked pull requests, defining persistence operation semantics, and verifying the migration workflow. ChangesConcurrency migration documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
- IPHONEOS_DEPLOYMENT_TARGET = 18.0 on app, Core, Widget and test targets; objectVersion 50 -> 56, LastUpgradeCheck -> 2600 (Xcode 26) - Remove availability guards now dead under the iOS 18 floor: iOS 11 placeholder fallback (PersonOverview), iOS 12 userInterfaceStyle guard (UIColor+Extensions), iOS 17 containerBackground shim (Widget) - Replace deprecated topLayoutGuide with safeAreaLayoutGuide - protocol X: class -> AnyObject across all protocols - Delete dead Obfuscator.swift plus its commented-out call sites and obfuscated constants in Logging/Constants - Readme requirements: iOS 18.0+ SWIFT_VERSION stays 5.0: Swift 6 language mode hinges on the concurrency direction discussed in PR #8, so it is deferred there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/concurrency-migration-plan.html`:
- Line 393: Update the 8A–8C guidance in the list item to explicitly instruct
creating draft PRs: either mark each PR as draft in the interactive gh stack
submit editor or use gh stack submit --auto without --open, reserving --open for
PRs intended to be ready for review.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c29b42d7-0b8a-43fc-86bb-4a475186f4ff
📒 Files selected for processing (2)
docs/concurrency-migration-plan.htmlmigrations/07-concurrency-direction.md
A decision doc (not an implementation ticket) laying out options for the GCD TaskManager: keep & fix, migrate to async/await, or hybrid. Implemen- tation ticket follows once a direction is chosen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1a5b577 to
a1374ec
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/concurrency-migration-plan.html`:
- Line 235: Update docs/concurrency-migration-plan.html at lines 235-235,
384-384, and 391-391: describe stacked pull requests as a public preview, retain
only the repository feature-availability check at the account-access
verification site, and state that the public-preview feature may change in the
risk text.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b5b9f288-a42c-40a3-a8d5-e4c112bd9481
📒 Files selected for processing (2)
docs/concurrency-migration-plan.htmlmigrations/07-concurrency-direction.md
1 participant
What
Adds
migrations/07-concurrency-direction.md— a discussion / decision doc, not an implementation ticket. Use this PR thread to pick a direction; an implementation ticket follows once a box is checked.The question
Background work goes through a hand-rolled
TaskManagerOnGCD(DispatchQueue+DispatchGroup). The current implementation is quietly buggy: it swallows thrown errors (try? task()), creates aDispatchGroupit never waits/notifies on, and itstypealias Tasknow clashes with Swift's_Concurrency.Task.Options
TaskManager(smallest change)TaskManagerRecommendation leans B, since the deployment-target bump (#5) and SwiftUI migration (#3) both pair naturally with async/await. Noting your stated preference not to keep DispatchGroup over async/await — this doc just confirms scope/sequencing before committing.
🤖 Generated with Claude Code
Summary by CodeRabbit