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

[ISSUE-009] 파이어베이스 크래시리틱스 추가 #28

Merged
merged 13 commits into from
May 30, 2022

Conversation

jihee-dev
Copy link
Member

@jihee-dev jihee-dev commented May 28, 2022

ISSUE


작업 내용

  • 파이어베이스 크래시리틱스 추가
    google-service.json 파일은 Growth 노션에 업로드 완료(브랜치 당겨온 뒤에 앱 수준 디렉토리에 해당 파일을 다운받아 옮겨 주세요!)
    • 해당 파일 포함해서 올렸습니다~

실행 화면

Screen Shot Screen Shot
스크린샷 2022-05-29 오전 2 14 56

Check List

  • PR 제목은 [TICKET-{N}] PR 제목으로 작성
  • CI/CD 통과 여부
  • 1명 이상의 Approve 확인 후 Merge
  • 관련 이슈 연결

@jihee-dev jihee-dev self-assigned this May 28, 2022
@jihee-dev
Copy link
Member Author

jihee-dev commented May 28, 2022

오.. 지금 이거 빌드 실패하는 이유가 google-service.json 파일을 찾지 못해서인 것 같은데 이 파일을 Public 레포에 올려도 괜찮을지 아시는 분 계신가요,,
혹은 요 파일 없이 빌드 통과하는 방법..?

파일 추가해서 커밋하는 것으로 해결

Comment on lines +53 to +55
implementation(platform(app.ModuleDependencies.FIREBASE_BOM))
implementation(app.ModuleDependencies.FIREBASE_ANALYTICS)
implementation(app.ModuleDependencies.FIREBASE_CRASHLYTICS)
Copy link
Member Author

Choose a reason for hiding this comment

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

이거 BOM으로 넣어주는 경우 버전 정보도 없고 platform으로 넣어주는 게 DependencyInfo 쪽에 없어서 그냥 이렇게 넣어줬는데 방법이 있을까요?

Copy link
Member

Choose a reason for hiding this comment

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

image

platform 이라는 워딩이 있긴 한데.. 어떻게 쓰는건지를 모르겠네요 저도 더 시도해볼게요

Comment on lines +129 to +131
const val FIREBASE_BOM = "com.google.firebase:firebase-bom:${Versions.FIREBASE_BOM}"
const val FIREBASE_ANALYTICS = "com.google.firebase:firebase-analytics-ktx"
const val FIREBASE_CRASHLYTICS = "com.google.firebase:firebase-crashlytics-ktx"
Copy link
Member Author

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

Choose a reason for hiding this comment

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

@jihee-dev @hoyahozz
버전없는 Data Class 만들어서 따로 주입해보기 해봤는데..

image

image

어림없네요 ㅋㅋㅋㅋㅋ
더 찾아봐야 할 것 같아요 방법이 있는지

일단 이렇게 가고 정호님 말씀대로 더 추가하게 되면 바꿔보죠
고생하셨습니다!
👍

Copy link
Member Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋ 넴 빌드 실패한 거 수정해서 머지할게용

Copy link
Member

@KwonDae KwonDae left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jihee-dev jihee-dev merged commit 887b839 into develop May 30, 2022
@jihee-dev jihee-dev deleted the feature/issue-009-firebase branch May 30, 2022 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[프로젝트 구축] Firebase 환경 추가
3 participants