You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
🟠 TOASTER-iOS/Application/Coordinator/Coordinator.swift — 1개 코멘트
Coordinator 프로토콜에 @MainActor를 추가하여 UI 관련 작업의 메인 스레드 안전성을 확보한 좋은 변경입니다. 기존 코디네이터 패턴의 명확한 구조와 팀 컨벤션 준수가 돋보입니다.
✅ Coordinators/ 하위의 구체적인 코디네이터들이 BaseCoordinator를 상속받고 final 키워드를 잘 사용하여 팀 컨벤션을 준수했습니다. / addDependency와 removeDependency 메서드를 통해 childCoordinators를 체계적으로 관리하는 방식이 좋습니다. 이는 코디네이터 계층 구조를 명확하게 하고, 의존성 관리에 용이합니다. / 전반적인 코디네이터 패턴 구현이 명확하고 견고하여, 앱의 내비게이션 및 흐름 제어 로직이 잘 분리되어 있습니다.
💡 프로젝트 구조를 보았을 때, 코디네이터 패턴이 앱의 전반적인 아키텍처에서 중요한 역할을 하고 있음을 알 수 있습니다. 이 변경은 코디네이터 계층의 메인 스레드 안전성을 강화하여, UI 관련 버그 발생 가능성을 줄이는 데 크게 기여할 것입니다. / ViewModel은 @MainActor로 선언이라는 팀 규칙이 있는 것으로 보아, UI 관련 로직의 메인 스레드 강제에 대한 팀의 강한 의지가 엿보입니다. 이 변경은 그 의지를 코디네이터 계층까지 확장하는 좋은 선례가 될 것입니다.
🟡 TOASTER-iOS/Application/Coordinator/Coordinators/TabBarCoordinator.swift — 1개 코멘트
코드 변경 사항을 검토한 결과, observeDeleteTokenEvent 메서드 내에서 onFinish 클로저를 호출하는 부분에 불필요한 Task { @MainActor in ... } 래핑이 추가되었습니다. NotificationCenter.default.addObserver의 queue: .main 파라미터 덕분에 해당 클로저 블록은 이미 메인 스레드에서 실행되므로, Task { @MainActor in ... }를 사용하는 것은 중복이며 제거할 수 있습니다. 전반적으로 Coordinator의 역할과 [weak self] 사용은 적절합니다.
✅ 클래스에 final 키워드를 사용하여 상속을 의도하지 않음을 명시하고 불필요한 오버라이딩을 방지했습니다. / 클로저 내에서 [weak self]를 사용하여 강한 참조 순환(retain cycle)을 효과적으로 방지했습니다. / 대부분의 프로퍼티를 private으로 선언하여 캡슐화를 잘 지키고 있습니다. / NotificationCenter의 queue: .main을 명시적으로 사용하여 UI 관련 로직이 메인 스레드에서 실행되도록 보장한 점은 좋습니다.
💡 TabBarCoordinator의 onFinish 클로저는 CoordinatorFinishOutput 프로토콜의 요구사항으로, 주로 상위 코디네이터 (AppCoordinator 등)에서 탭바 흐름이 종료될 때(예: 토큰 만료 시 로그인 화면으로 전환) 앱의 전체적인 흐름을 제어하는 데 사용됩니다. 따라서 onFinish가 메인 스레드에서 실행되어야 하는 것은 적절합니다. / 기존 코드(self?.onFinish?())도 NotificationCenter.default.addObserver(..., queue: .main) 덕분에 메인 스레드에서 실행되고 있었으므로, 이번 변경은 기능적인 차이를 만들지 않습니다.
📋 심각도 기준 · 감도 변경
심각도
의미
🔴 P1
꼭 반영
🟠 P2
적극 고려
🟡 P3
웬만하면 반영
🔵 P4
넘어가도 OK
💬 P5
사소한 의견
감도 변경: 워크플로우의 sensitivity 값 또는 .codereview.yml 파일에서 설정
🟠 TOASTER-iOS/Application/Coordinator/Coordinator.swift — 1개 코멘트
Coordinator 프로토콜에 @MainActor 추가는 코디네이터 패턴의 동시성 관리 및 데이터 레이스 방지를 위한 긍정적인 변경입니다. UI 관련 흐름을 제어하는 코디네이터의 역할에 적합하며, 팀의 ViewModel @MainActor 규칙과도 일관성을 이룹니다.
✅ @MainActor 추가로 Coordinator 프로토콜을 따르는 모든 코디네이터 인스턴스와 그 메서드 및 프로퍼티 접근이 메인 액터로 격리되어 동시성 안전성을 높입니다. / childCoordinators 배열과 같은 가변 상태에 대한 데이터 레이스 발생 가능성을 컴파일 타임에 예방할 수 있습니다. / UI 관련 로직을 메인 스레드에서 처리해야 한다는 의도를 명확히 하여 코드 가독성과 유지보수성을 향상시킵니다. / ViewModel에 @MainActor를 선언하는 팀 규칙과 일관성을 유지하여 프로젝트 전반의 동시성 모델을 통일시킵니다.
💡 모든 하위 코디네이터들(예: HomeCoordinator, LoginCoordinator)은 BaseCoordinator를 상속하고 Coordinator 프로토콜을 따르므로, 이 변경으로 인해 이들 코디네이터의 모든 메서드 호출 및 프로퍼티 접근이 자동으로 메인 액터에서 이루어지게 됩니다. / 코디네이터는 UI Navigation 스택을 관리하는 경우가 많으므로, 메인 액터에서 동작하도록 강제하는 것은 일반적인 패턴이며 적절한 설계입니다.
🟠 TOASTER-iOS/Application/Coordinator/CoordinatorFinishOutput.swift — 1개 코멘트
CoordinatorFinishOutput 프로토콜에 @MainActor를 추가하여 MainActor 격리를 명시적으로 적용했습니다. 이는 코디네이터가 주로 UI flow를 관리하고 MainActor에서 작동해야 하는 특성을 고려할 때 적절한 변경으로 판단됩니다. onFinish 클로저의 호출 및 할당이 MainActor에서 이루어지도록 강제하여 잠재적인 데이터 레이스를 방지하고 UI 관련 작업의 안전성을 높입니다.
✅ Coordinators의 UI 관련 역할과 MainActor의 필요성을 잘 이해하고 적용한 좋은 컨커런시 개선입니다. / 잠재적인 데이터 레이스를 예방하고 코드의 안정성을 높이는 변경입니다.
💡 프로젝트 내의 여러 Coordinator 클래스(HomeCoordinator, TimerCoordinator, ClipCoordinator, LoginCoordinator, AddLinkCoordinator)들이 이 CoordinatorFinishOutput 프로토콜을 채택하고 있으며, 모두 onFinish 클로저를 통해 부모 코디네이터에게 작업 완료를 알리는 용도로 사용됩니다. 이 클로저를 통한 통신이 UI 관련 작업을 수반할 가능성이 높으므로, @MainActor를 적용하는 것은 전반적인 UI 상태 관리의 일관성과 안전성을 확보하는 데 중요합니다.
📋 심각도 기준 · 감도 변경
심각도
의미
🔴 P1
꼭 반영
🟠 P2
적극 고려
🟡 P3
웬만하면 반영
🔵 P4
넘어가도 OK
💬 P5
사소한 의견
감도 변경: 워크플로우의 sensitivity 값 또는 .codereview.yml 파일에서 설정
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 해결한 이슈
🛠️ 작업내용
Swift 6 Migration 진행했습니다요.
https://medium.com/@mini-min/swift-swift-6-%EB%A7%88%EC%9D%B4%EA%B7%B8%EB%A0%88%EC%9D%B4%EC%85%98-%EC%A0%9C%EA%B0%80-%ED%95%9C%EB%B2%88-%ED%95%B4%EB%B3%B4%EC%95%98%EC%8A%B5%EB%8B%88%EB%8B%A4-26dced5127e6
📂 참고한 내용
✅ Checklist