-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: ContentSettingFeature 액션 컨벤션 수정 #141
Conversation
.run { send in | ||
for await _ in self.pasteboard.changes() { | ||
let url = try await pasteboard.probableWebURL() | ||
await send(.inner(.linkPopup(url)), animation: .pokitSpring) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
얘도 액션으로 빼서 깔끔하게 가져갈 수 있읍니다
let isEdit = state.domain.categoryId != nil | ||
|
||
if isEdit { | ||
return .send(.async(.컨텐츠_수정_API)) | ||
} else { | ||
return .send(.async(.컨텐츠_추가_API)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isEdit
? return .action
: return .action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰하려고 했는데 보다가 포기했습니다.
concatenate, merge 사용하시는 부분들 전부 가독성 너무 떨어지는 것 같습니다.
순차적으로 혹은 순서와 상관없이 액션을 실행하고 싶은 마음은 충분히 이해는 합니다. 그러면은
주석을 달아주시던지 함수로 빼보시던지 PR로 해당 내용을 소개해주시던지 하나는 해주셔야 된다고 생각합니다.
읽는사람도 생각해주세요
넵..!! 너무 생각없이 적었습니다. 주의하겠습니다..!! 함수로 빼는 방향으로 가겠습니다 |
#️⃣연관된 이슈
📝작업 내용
스크린샷 (선택)
💬리뷰 요구사항(선택)
close 이슈번호