Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

Conversation

@marandaneto
Copy link
Contributor

@marandaneto marandaneto commented Aug 24, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

fix: add main thread checker for the app lifecycle integration
this will make the App. to not crash it, but still won't be able to add the observer, see next steps.

💡 Motivation and Context

#520

💚 How did you test it?

unit test and running on older androidx versions.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

maybe execute the addObserver on the main Looper

@marandaneto marandaneto requested a review from a team August 24, 2020 13:20
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, maybe we can improve a few things though.

@billnbell
Copy link

Will this be a release to npm ?

@bruno-garcia
Copy link
Member

Will this be a release to npm ?

Once we release the Android SDK, we bump the react-native SDK's dependency

@haelsisy
Copy link

When this PR will be released?

@marandaneto marandaneto requested review from a team and philipphofmann August 25, 2020 08:26
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM

@marandaneto
Copy link
Contributor Author

When this PR will be released?

I need to sync with the RN SDK, but most probably this week (early as possible) as we know this is a blocker for some users.

@marandaneto marandaneto merged commit 5da4269 into getsentry:master Aug 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants