Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the codebase to Swift 6 strict concurrency mode by adding nonisolated annotations, enabling new Swift 6.2 features, and improving concurrency-related comments and documentation.
Changes:
- Enabled Swift 6 concurrency features including
SWIFT_DEFAULT_ACTOR_ISOLATION = MainActorandSWIFT_UPCOMING_FEATURE_NONISOLATEDNONSENDINGBYDEFAULT - Added
nonisolatedannotations to test classes, enums, structs, and static members - Improved comments explaining
Task.detachedusage for off-MainActor I/O operations - Added comprehensive Swift Concurrency reference documentation
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Kaset.xcodeproj/project.pbxproj | Added Swift 6 concurrency build settings |
| .swiftformat | Added nonisolated to modifier order |
| Tests/KasetUITests/*.swift | Marked UI test classes as nonisolated |
| Tests/KasetUITests/KasetUITestCase.swift | Added extensive nonisolated annotations to test helpers |
| Tests/KasetTests/Helpers/MockURLProtocol.swift | Added nonisolated annotations and improved comments |
| Core/Models/*.swift | Added nonisolated to model structs and enums |
| Core/Utilities/*.swift | Added nonisolated to utility enums and updated comments |
| Core/Services/*.swift | Improved Task.detached comments |
| .github/skills/swift-concurrency/* | Added comprehensive Swift Concurrency reference documentation |
Comments suppressed due to low confidence (1)
Core/Utilities/DiagnosticsLogger.swift:1
- Redundant
nonisolatedon static let constants within a nonisolated enum. Immutable static properties within nonisolated types are inherently nonisolated.
import Foundation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| nonisolated enum TestAccessibilityID { | ||
| nonisolated enum Sidebar { | ||
| nonisolated static let container = "sidebar" | ||
| nonisolated static let searchItem = "sidebar.search" |
There was a problem hiding this comment.
Redundant nonisolated annotations on nested types and static properties. With SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor enabled, these enums and their static string constants are inherently nonisolated (immutable value types). The outer nonisolated on the enum is sufficient; inner annotations add noise without providing value.
| nonisolated struct MockFavoriteItem { | ||
| let id: String | ||
| let pinnedAt: Date | ||
| let type: MockFavoriteType | ||
|
|
||
| enum MockFavoriteType { | ||
| nonisolated enum MockFavoriteType { |
There was a problem hiding this comment.
Redundant nonisolated on nested enum within a struct. Value types (structs/enums) with immutable properties are inherently nonisolated. Only the struct-level annotation is needed.
|
|
||
| /// Creates a mock song favorite. | ||
| static func song(videoId: String, title: String, artist: String) -> MockFavoriteItem { | ||
| nonisolated static func song(videoId: String, title: String, artist: String) -> MockFavoriteItem { |
There was a problem hiding this comment.
Redundant nonisolated on static factory methods within an already nonisolated type. Static methods on nonisolated types inherit the isolation.
Core/Models/Song.swift
Outdated
|
|
||
| /// Represents a song/track from YouTube Music. | ||
| struct Song: Identifiable, Codable, Hashable, Sendable { | ||
| nonisolated struct Song: Identifiable, Codable, Hashable, Sendable { |
There was a problem hiding this comment.
Redundant nonisolated annotation. Value types conforming to Sendable are implicitly nonisolated. This annotation is unnecessary and adds visual clutter.
Core/Models/Song.swift
Outdated
| nonisolated static func == (lhs: Song, rhs: Song) -> Bool { | ||
| // Compare by video ID for identity equality | ||
| lhs.videoId == rhs.videoId | ||
| } | ||
|
|
||
| func hash(into hasher: inout Hasher) { | ||
| nonisolated func hash(into hasher: inout Hasher) { | ||
| hasher.combine(self.videoId) | ||
| } |
There was a problem hiding this comment.
Redundant nonisolated on protocol requirement implementations. Equatable and Hashable protocol requirements are inherently nonisolated, as they must be callable from any context.
| nonisolated enum AccessibilityID { | ||
| // MARK: - Sidebar | ||
|
|
||
| enum Sidebar { | ||
| nonisolated enum Sidebar { | ||
| static let container = "sidebar" |
There was a problem hiding this comment.
Redundant nonisolated on nested enums. The outer enum annotation is sufficient; nested types inherit nonisolation.
| // swiftlint:disable:next force_try | ||
| let data = try! JSONSerialization.data(withJSONObject: json) | ||
| self.requestHandler = { request in | ||
| Self.requestHandler = { request in |
There was a problem hiding this comment.
Inconsistent use of Self vs self. Line 67 uses Self.requestHandler while line 81 uses self.requestHandler for the same pattern. Prefer consistent capitalization.
Description
AI Prompt (Optional)
🤖 AI Prompt Used
AI Tool:
Type of Change
Related Issues
Changes Made
Testing
xcodebuild test -only-testing:KasetTests)Checklist
swiftlint --strict && swiftformat .Screenshots
Additional Notes