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

앱팀 신규 피처 - 콕 찌르기 #545

Merged
merged 140 commits into from
Jan 23, 2024
Merged

앱팀 신규 피처 - 콕 찌르기 #545

merged 140 commits into from
Jan 23, 2024

Conversation

kimdahyee
Copy link
Contributor

앱팀 신규 피처 - 콕 찌르기 기능 구현
by @yxnsx @kimdahyee

- 누가 나를 찔렀어요
- 내 친구를 찔러보세요
- 누가 나를 찔렀어요
- 내 친구를 찔러보세요
@kimdahyee kimdahyee self-assigned this Jan 22, 2024
@kimdahyee kimdahyee requested a review from a team as a code owner January 22, 2024 15:30
Copy link
Contributor

@hjh1161514 hjh1161514 left a comment

Choose a reason for hiding this comment

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

양이 정말 많네요.. 너무너무 수고하셨습니다!

<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:shape="rectangle">
<corners android:radius="6dp" />
<solid android:color="@color/mds_gray_700" />
Copy link
Contributor

Choose a reason for hiding this comment

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

border가 따로 없고 solid로만 색상을 사용한다면 기존에 있는 rectangle_radius_6를 사용하면 어떨까요??
색상에 따라 모든 drawable을 생성하는 것이 아니라 radius만 고정으로 두고 다양한 색상에서 사용할 수 있도록 하는 게 좋을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 !!! 다음에 수정해서 나갈 수 있도록 하겠습니다 !!

Copy link
Member

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

고생많았습니다..!!

@@ -35,6 +35,7 @@ plugins {
alias(libs.plugins.secret)
alias(libs.plugins.sentry)
alias(libs.plugins.app.distribution)
alias(libs.plugins.kotlin.android)
Copy link
Member

Choose a reason for hiding this comment

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

이거 넣으신 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 ... 깃을 보니 저네요 ... 가끔 저도 모르게 저렇게 플러그인이 추가되던데 .. 이유가 뭘런지 ..

@@ -91,7 +91,7 @@ class AlertDialogOneButton(context: Context) : ConstraintLayout(context) {
dialog?.show()
}

fun dismiss() {
private fun dismiss() {
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

Comment on lines +35 to +36
private val soptampLocal: SoptampDataStore,
private val pokeLocal: PokeLocalDataSource,
Copy link
Member

Choose a reason for hiding this comment

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

pokelocal 활용하는 곳이 없는 것 같습니다..? 혹시 추가하신 이유 있을까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

pokeLocal에 온보딩 첫 진입 여부 값을 저장하고 있는데 UserRepository에서는 로그아웃시 이를 초기화하는 작업만 선언해두었습니다!

Comment on lines 27 to 30
data class PokeUserRequest(
val userId: Int,
val message: String,
)
Copy link
Member

Choose a reason for hiding this comment

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

Request 객체에 Serializable 같은거 안 붙여도 되나요..?

Copy link
Contributor

Choose a reason for hiding this comment

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

적용 완..!!!!

Copy link
Member

Choose a reason for hiding this comment

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

도메인 모듈 설계에서의 본래의도는 외부/내부 데이터소스의 응답과는 무관하게 코틀린 객체만으로 순수하게 비즈니스 로직을 구성하기 위함인데 ApiResult와 BaseResponse 같은 객체의 설계는 이러한 순수성을 떨어트리는 것 같긴합니다만...시간이 워낙 촉박했던지라 이와 같은 설계가 나오게 된 것은 공감하는 부분입니다.

Comment on lines +56 to +58
private var totalPageSize = -1
private var currentPaginationIndex = 0
private var friendListJob: Job? = null
Copy link
Member

Choose a reason for hiding this comment

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

오....수동페이징 👍🏻

}
}

inner class ViewHolder(
Copy link
Member

Choose a reason for hiding this comment

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

inner modifier 제거해도 괜찮을 것 같습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

제거 완..!!!

Copy link
Member

Choose a reason for hiding this comment

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

폼 미쳤다...!!

Copy link
Member

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

오랜 기간 작업하시느라 고생많으셨습니다 👍🏻

@yxnsx yxnsx merged commit db09b82 into develop Jan 23, 2024
1 check passed
@yxnsx yxnsx deleted the feature/yxnsx/#500-poke branch January 23, 2024 14:36
@l2hyunwoo l2hyunwoo restored the feature/yxnsx/#500-poke branch January 23, 2024 14:39
@l2hyunwoo l2hyunwoo deleted the feature/yxnsx/#500-poke branch January 23, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants