Skip to content

ADD Crashlytics #23

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

Merged
merged 14 commits into from
Apr 5, 2024
Merged

ADD Crashlytics #23

merged 14 commits into from
Apr 5, 2024

Conversation

ninovanhooff
Copy link
Collaborator

@ninovanhooff ninovanhooff commented Aug 11, 2023

Why is this important?

All projects probably use crashlytics

@ninovanhooff
Copy link
Collaborator Author

@Frank1234 do you still object to having a Firebase project and google-services.json in this template?

@Frank1234 Frank1234 added the changes requested This doesn't seem right label Sep 28, 2023
Copy link
Collaborator

@Frank1234 Frank1234 left a comment

Choose a reason for hiding this comment

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

Let's just add them! Without the stubs. Just some q template project id with a bit fat logged wraning on startup. We always use crashlytics anyways.

@ninovanhooff ninovanhooff changed the title ADD stubs for Crash logging ADD Crashlytics Dec 1, 2023
# Conflicts:
#	app/src/main/kotlin/nl/q42/template/MainApplication.kt
#	feature/home/src/main/kotlin/nl/q42/template/presentation/home/HomeViewModel.kt
#	gradle/libs.versions.toml
@ninovanhooff ninovanhooff added ready for review and removed changes requested This doesn't seem right labels Dec 15, 2023
Copy link
Collaborator

@sebaslogen sebaslogen left a comment

Choose a reason for hiding this comment

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

I'm fine with adding Crashlytics by default but not with adding Analytics by default.

AFAIK, this is not GDPR complaint and that seems a dangerous think to forget when using a template.
Is it possible to leave analytics off by default, at least the parts that are non-GDPR compliant? And then add some doc on how to enable if you really need them.

because it might not be GDPR compliant

This allows reverse proxies like Charles to inspect and manipulate traffic in development builds
sebaslogen
sebaslogen previously approved these changes Dec 15, 2023
Copy link
Collaborator

@Frank1234 Frank1234 left a comment

Choose a reason for hiding this comment

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

sorry for the late check! a few change requests.

# Conflicts:
#	gradle/libs.versions.toml
sebaslogen
sebaslogen previously approved these changes Feb 23, 2024
sebaslogen
sebaslogen previously approved these changes Mar 22, 2024
Techwolf12
Techwolf12 previously approved these changes Mar 22, 2024
# Conflicts:
#	feature/home/src/main/kotlin/nl/q42/template/presentation/home/HomeViewModel.kt
Frank1234
Frank1234 previously approved these changes Mar 26, 2024
Copy link
Collaborator

@Frank1234 Frank1234 left a comment

Choose a reason for hiding this comment

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

approved with 1 comment!

@ninovanhooff ninovanhooff dismissed stale reviews from Frank1234, Techwolf12, and sebaslogen via 191ba61 April 5, 2024 14:21
@ninovanhooff ninovanhooff merged commit 766c705 into main Apr 5, 2024
1 check passed
@ninovanhooff ninovanhooff deleted the crash-logging branch April 5, 2024 14:30
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.

5 participants