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] Splash, SignIn, SignUp 로깅 세팅 #197

Merged
merged 14 commits into from
Nov 13, 2023
Merged

Conversation

KxxHyoRim
Copy link
Member

  • splash, siginIn, signUp 과정에 대한 logging 입니다.

To 멘토님

  • LoggingScheme이 추가될때마다 Builder 를 만드는게 번거롭게 느껴졌습니다.
  • 이에 대한 대안으로 SwmCommonLoggingScheme을 만들어 재활용했습니다.
  • SwmCommonLoggingScheme는 backend api에 만들때와 같이 map 형태를 활용합니다
  • Scheme class를 만들때 하드코딩으로 지정해주었던 ScreenName, EventNameBuilder를 활용해 지정하게끔 변경했습니다.
before after
image image

@KxxHyoRim KxxHyoRim requested a review from acious November 13, 2023 20:22
@KxxHyoRim KxxHyoRim self-assigned this Nov 13, 2023
@KxxHyoRim KxxHyoRim merged commit 9345419 into develop Nov 13, 2023
1 check passed
Comment on lines +14 to +19
setLoggingScheme(
eventLogName = eventLogName,
screenName = screenName,
logVersion = logVersion,
logData = logData.toMutableMap()
)
Copy link

Choose a reason for hiding this comment

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

이 메소드는 어디에 속한 메소드인가요?

Comment on lines +123 to +129
private fun shotSignInExposureLogging() {
val scheme = SwmCommonLoggingScheme.Builder()
.setEventLogName("signInExposure")
.setScreenName(this.javaClass)
.build()
signInViewModel.shotSignInExposureLogging(scheme)
}
Copy link

Choose a reason for hiding this comment

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

모든 액티비티마다 이러한 로깅을 쏘게된다면 BaseActivity에 묻어서 로깅을 처리할 방법은 없을지도 생각해보면 좋을거같아요.

@acious
Copy link

acious commented Nov 26, 2023

미뤄두었던 요청하신 마지막 코드리뷰 진행했습니다 ㅎㅎ
1년동안 정말 수고많았고 열심히 함께 해주셔서 감사합니다 ㅎㅎ

내년에는 새로운 시간과 주제로 또 같이 많이 이야기해보시죠

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.

2 participants