[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

Moved Fabric initialization up #9564

Merged
merged 1 commit into from
Apr 9, 2019
Merged

Moved Fabric initialization up #9564

merged 1 commit into from
Apr 9, 2019

Conversation

mzorz
Copy link
Contributor
@mzorz mzorz commented Apr 9, 2019

h/t @hypest who first suggested this could be a problem, as we were getting reports of crashes when the app starts and could not see anything on Fabric.

This PR moves the initialization code for Fabric up, as the first thing on onCreate() to also get crashes with potential SQL initialization

To test:

  1. try to make the app crash (maybe by artificially adding an exception in code temporarily, or by trying to reproduce an existing known crash)
  2. observe crashes are sent to Fabric

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@mzorz mzorz added the Analytics label Apr 9, 2019
@mzorz mzorz added this to the 12.3 milestone Apr 9, 2019
@mzorz mzorz requested review from hypest and jkmassel April 9, 2019 18:13
jkmassel
jkmassel approved these changes Apr 9, 2019
View reviewed changes
Copy link
Contributor
@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

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

I have no concerns with this – these components are independent of one another, so the order shouldn't matter.

@jkmassel jkmassel merged commit d8ff1a6 into develop Apr 9, 2019
@aforcier
Copy link
Contributor
aforcier commented Apr 9, 2019

cc @AmandaRiu @nbradbury probably worth doing this for WooAndroid as well

@jkmassel jkmassel deleted the fix/init-fabric-before-wellsql branch April 9, 2019 18:29
@nbradbury nbradbury mentioned this pull request Apr 15, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkmassel jkmassel jkmassel approved these changes

@hypest hypest Awaiting requested review from hypest

Assignees
No one assigned
Labels
Analytics
Projects
None yet
Milestone
12.3
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mzorz @aforcier @jkmassel