[Team-05 iOS JEJE, 만사] OAuth 로그인 화면 & Issue 전체 조회 화면 #27
Open
ITzombietux wants to merge 50 commits intocodesquad-members-2021:team5from
Open
[Team-05 iOS JEJE, 만사] OAuth 로그인 화면 & Issue 전체 조회 화면 #27ITzombietux wants to merge 50 commits intocodesquad-members-2021:team5from
ITzombietux wants to merge 50 commits intocodesquad-members-2021:team5from
Conversation
[iOS] Issue & Login 구현
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
ehgud0670
pushed a commit
that referenced
this pull request
Jun 17, 2021
Conflicts: iOS/issue-tracker/issue-tracker.xcodeproj/project.pbxproj
|
안녕하세요. 일이 바쁜거랑 개인일이 겹쳐서 PR을 늦게 보게 되서 죄송합니다. 일 끝나고 오늘 저녁에 진행할 수 있도록 하겠습니다. 늦어져서 죄송합니다🙏 |
mienne
approved these changes
Jun 22, 2021
There was a problem hiding this comment.
- 강제 옵셔널 코드가 보였습니다. 코드 작성할 때 사용한 의도는 있다고 생각합니다. 그러나, 강제 옵셔널 사용하지 않고 해결할 수 있는 방법이 있다면 그쪽으로 풀어가는 것이 좋습니다.
- 사용하지 않는 코드에 대해서 따로 코멘트 남기지 않았습니다. 보이는대로 정리해주세요!
- 여유가 있을 때 API 응답 실패, 성공 케이스에 대해서 응답 받을 때, Presentation 영역에서 어떻게 처리할지 고민하면 좋을거 같아요!
- 추상화에 대해서 저도 배울 부분이 많이 있었습니다. 잘하셨습니다!!👏
| func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool { | ||
|
|
||
| NotificationCenter.default.addObserver(forName: ASAuthorizationAppleIDProvider.credentialRevokedNotification, object: nil, queue: nil) { (notification) in | ||
| print("로그인 페이지로 이동") |
| import Foundation | ||
|
|
||
| struct IssueEndPoint { | ||
| static let scheme = "https" |
| class IssueNetworkController { | ||
| private var requests: [URL: AnyObject] = [:] | ||
|
|
||
| init() { |
| } | ||
|
|
||
| func fetchIssues(completion: @escaping ([Issue]) -> Void) { | ||
| let req = IssueRequest() |
There was a problem hiding this comment.
fetchIssues, deleteIssue 호출할 때마다 Request 객체 매번 생성하는 것보다 IssueNetworkController 생성하는 시점에서 한 번만 생성해서 사용해도 무방할거 같은데, 이 부분은 의견이 궁금합니다!
| req.execute { (result) in | ||
| if let _ = result { | ||
| self.requests[requestURL] = nil | ||
| completion(result!) |
There was a problem hiding this comment.
result! 강제 언래핑보다 if let 옵셔널 해제한 것으로 사용하세요🙂
| // MARK: - HTTPStatusRequest | ||
| protocol HTTPStatusRequest: NetworkRequest {} | ||
|
|
||
| extension HTTPStatusRequest { |
There was a problem hiding this comment.
현재 사용하지 않는거 같은데, HTTPStatusRequest 추상화한 목적이 무엇인지 궁금합니다!
| return nil | ||
| } | ||
| let decoder = JSONDecoder() | ||
| decoder.dateDecodingStrategy = .iso8601 |
There was a problem hiding this comment.
만약 IssueRequest, IssuePatchRequest에서 dateDecodingStrategy 각각 다른 값으로 변경되어야 한다면 어떻게 대응해야 할까요?
|
|
||
| import UIKit | ||
|
|
||
| extension UITableView { |
There was a problem hiding this comment.
UITableView에서 어떤 것을 extension 한건지 알 수 있도록 파일명에서 드러나면 좋을거 같아요. 현재 같이 재사용과 관련된 부분이기 때문에 UITableView+Reusable 같이 작성하면 좋겠네요🙂
| import UIKit | ||
|
|
||
| // MARK: Protocols | ||
| protocol Coordinator: class { |
There was a problem hiding this comment.
Coordinator 파일, 코드 위치가 Domain Layer 보다 View와 연관있기 때문에 Presentation Layer로 이동해야겠네요.
| self.layer.masksToBounds = true | ||
| } | ||
|
|
||
| required init?(coder: NSCoder) { |
There was a problem hiding this comment.
지원하지 않는다면 아래와 같이 사용한다면 컴파일에서 경고나 오류로 표시할 수 있습니다. 참고만 해주세요!
Suggested change
| required init?(coder: NSCoder) { | |
| @available(*, unavailable, message: "TagLabel는 코드로만 생성할 수 있습니다.") | |
| required init?(coder: NSCoder) { |
GangWoon
pushed a commit
that referenced
this pull request
Jul 20, 2021
GangWoon
pushed a commit
that referenced
this pull request
Jul 20, 2021
GangWoon
pushed a commit
that referenced
this pull request
Jul 20, 2021
GangWoon
pushed a commit
that referenced
this pull request
Jul 20, 2021
GangWoon
pushed a commit
that referenced
this pull request
Jul 20, 2021
GangWoon
pushed a commit
that referenced
this pull request
Jul 20, 2021
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
목표
구현 기능
로그인, 이슈 등 화면 이동과 객체 상태 공유를 Coodinator를 사용하여 구현
OAuth GitHub 기능 구현