Audio #88
Closed
Audio #88
Conversation
This file contains 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
…ookcover 홈화면 책커버 Drag & Drop 구현
This reverts commit 44d80d2.
Kyxxn
requested changes
Nov 30, 2024
MemorialHouse/MHPresentation/MHPresentation/Source/Audio/AudioViewModel.swift
Outdated
Show resolved
Hide resolved
MemorialHouse/MHPresentation/MHPresentation/Source/Audio/AudioViewModel.swift
Show resolved
Hide resolved
MemorialHouse/MHPresentation/MHPresentation/Source/Audio/AudioViewModel.swift
Outdated
Show resolved
Hide resolved
k2645
approved these changes
Nov 30, 2024
Comment on lines
180
to
185
| case .updatedAudioFileURL: | ||
| debugPrint("updated audio file URL") | ||
| case .savedAudioFile: | ||
| debugPrint("saved audio file") | ||
| case .deleteTemporaryAudioFile: | ||
| debugPrint("delete temporary audio file") |
There was a problem hiding this comment.
P2: 해당 부분은 output에 대한 로직이 따로 존재하지않는다면 굳이 output을 방출하지 않아도 되지않을까.. 싶습니다 ! 혹시 이후 이 부분에 추가적인 로직이 필요한 것이라면 TODO로 각각의 output에 대해 어떤 로직이 추가되어야할지 적어두는게 좋을 것 같습니다 !
추가로 아래 효준님 코멘트와 같이 여기도 MHLogger로 변경해주면 좋을 것 같아용
There was a problem hiding this comment.
todo 간단하게 작성할게요 ~
iceHood
approved these changes
Nov 30, 2024
Comment on lines
+137
to
+139
| let fileName = "\(identifier).m4a" | ||
| let documentDirectory = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] | ||
| let audioFileURL = documentDirectory.appendingPathComponent(fileName) |
There was a problem hiding this comment.
P2. 이름관련해서 AudioViewController, AudionViewModel이면 추후에 저희가 보여질 뷰와 이름이 겹칠것같아서 AddAudio-- 나 EditAudio-- CreateAudio-- 등을 쓰는 것은 어떨까요?
| // timer | ||
| private var recordingSeconds: Int = 0 | ||
| private var recordingTimer: Timer? | ||
| private var timeTimer: Timer? |
There was a problem hiding this comment.
P3. timeTimer는 혹시 어떤 역할인가요? (이름만으로 파악하기 힘든 것같아요)
There was a problem hiding this comment.
레코딩 시간을 의미하는 것이였는데, 다시보니 이해가 잘 안되서 수정했습니다.
There was a problem hiding this comment.
P3. 나중에 UseCase를 통해 파일 관련 로직을 위임하는 것도 좋을 것같습니다!
|
commit count issue |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
#️⃣ 연관된 이슈
⏰ 작업 시간
저는 왕 감자 입니다. 🥔🥔🥔
📝 작업 내용
📸 스크린샷
ScreenRecording_11-27-2024.21-03-56_1.MP4
📒 리뷰 노트
⚽️ 트러블 슈팅
런타임에 오토레이아웃이 조절되지 않는 문제
constraints를 deactivate해주고 다시 activate하는 방식으로 해결