Skip to content
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

feat: 쪽지 관련 공통 코드 작성 #450

Merged
merged 12 commits into from
Oct 12, 2023

Conversation

krrong
Copy link
Collaborator

@krrong krrong commented Oct 11, 2023

📌 관련 이슈

🛠️ 작업 내용

  • Repository 및 도메인 클래스 생성
  • DTO 생성
  • Service 생성
  • Hilt 모듈에 추가

🎯 리뷰 포인트

  • 추가된 코드 중 불필요한 요소 혹은 더 필요한 요소가 있을지 확인해주시면 좋을 것 같아요!
  • DTO에 대해 백엔드와 조금 더 협의가 필요한 부분이 있어보여서 해당 부분 회의에서 조율해봐요!

⏳ 작업 시간

추정 시간: 1h
실제 시간: 2h
어렵네요.. 도메인, DTO 짜는거..

Copy link
Collaborator

@hyunji1203 hyunji1203 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

크롱! 밤에 공통 코드 짜느라 고생 많았어요!!🦖
몇가지 수정해야 할 부분에 대해서 리뷰 남겼으니까 확인해주시면 됩니다!
공통 코드 쪽지 개발하면서 야무지게 잘 사용하도록 하겠습니다👍ㅋㅋㅋㅋ

Comment on lines 21 to 22
@Query("latitude") latitude: String,
@Query("longitude") longitude: String,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@Query("latitude") latitude: String,
@Query("longitude") longitude: String,
@Query("latitude") latitude: Double,
@Query("longitude") longitude: Double,

노션을 보니 해당 쿼리 타입이 Double인 것 같아요!
아니면 String으로 해도 되는건가요?! 몰라서 물어봅니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다!

TODO("Not yet implemented")
}

override suspend fun fetchNearbyLetters(latitude: String, longitude: Coordinate): List<ClosedLetter> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
override suspend fun fetchNearbyLetters(latitude: String, longitude: Coordinate): List<ClosedLetter> {
// 1번 방식
override suspend fun fetchNearbyLetters(latitude: Double, longitude: Double): List<ClosedLetter> {
// 2번 방식
override suspend fun fetchNearbyLetters(coordinate: Coordinate): List<ClosedLetter> {

이렇게 바뀌어야 할 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했어요!

longitude: Coordinate,
): OpenLetter

suspend fun fetchNearbyLetters(latitude: String, longitude: Coordinate): List<ClosedLetter>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위의 리뷰와 마찬가지 입니다!🕺


import com.now.domain.model.type.LogType

data class LetterLog(
Copy link
Collaborator

@hyunji1203 hyunji1203 Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 도메인은 더 이상 사용 되고 있지 않는 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

삭제했습니다!

Copy link
Collaborator

@hyunji1203 hyunji1203 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM~🦖
고생 많았어요! 굳굳!!!

@hyunji1203 hyunji1203 merged commit 0404224 into dev_android Oct 12, 2023
@hyunji1203 hyunji1203 deleted the feat/#448_쪽지_관련_공통_코드_작성 branch October 12, 2023 08:55
@krrong krrong mentioned this pull request Oct 17, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants