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

Add enableCrashlyticsCollection to Crashlytics #1718

Merged
merged 19 commits into from
Dec 6, 2018

Conversation

timwangdev
Copy link
Contributor

@timwangdev timwangdev commented Nov 29, 2018

Summary

Allow developer to manually initialize Crashlytics when disabled automatic initialization as described here. If the the module has already been initialized, this method should be a no-op according to Fabric docs.

This would be useful in cases like GDPR.

Checklist

Test Plan

Pass e2e test.

Release Plan

[IOS] [ANDROID] [FEATURE] [CRASHLYTICS] - Add enableCrashlyticsCollection to Crashlytics


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@timwangdev
Copy link
Contributor Author

CI reported eslint is not found, you may want to have a look.

@Salakar Salakar self-requested a review December 1, 2018 12:52
@Salakar Salakar added this to the v5.2.0 milestone Dec 1, 2018
Salakar
Salakar previously approved these changes Dec 1, 2018
increment yarn caching version - eslint bug
@codecov
Copy link

codecov bot commented Dec 1, 2018

Codecov Report

Merging #1718 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1718      +/-   ##
==========================================
+ Coverage    76.1%   76.16%   +0.06%     
==========================================
  Files          72       72              
  Lines        1837     1838       +1     
==========================================
+ Hits         1398     1400       +2     
+ Misses        439      438       -1

1 similar comment
@codecov
Copy link

codecov bot commented Dec 1, 2018

Codecov Report

Merging #1718 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1718      +/-   ##
==========================================
+ Coverage    76.1%   76.16%   +0.06%     
==========================================
  Files          72       72              
  Lines        1837     1838       +1     
==========================================
+ Hits         1398     1400       +2     
+ Misses        439      438       -1

@Salakar
Copy link
Member

Salakar commented Dec 1, 2018

Thanks for this @timwangdev - looks great. Looks like the eslint issue is something to do with yarn caching on the ci, I've just pushed a commit on your branch to increment the cache version to wipe it, should be ok now - we'll see 👍

image

Salakar
Salakar previously approved these changes Dec 1, 2018
Salakar pushed a commit to invertase/react-native-firebase-docs that referenced this pull request Dec 1, 2018
@Salakar
Copy link
Member

Salakar commented Dec 1, 2018

Well, I can't get the CI to pod install will take another look at this tomorrow 😪

@Salakar Salakar merged commit 1ab8439 into master Dec 6, 2018
@Salakar Salakar deleted the disable-crash-collection branch December 6, 2018 17:39
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