segment-integrations / analytics-ios-integration-firebase

Segment's bundled integration for Firebase on iOS
MIT License
13 stars 92 forks source link

Fix: crash on FIRApp configure #37

Closed kanclalg closed 3 years ago

kanclalg commented 5 years ago

Fix for #36

m-vdb commented 5 years ago

@vargero if we manage to do a fix and follow your recommendations, how soon can this be merged?

vargero commented 5 years ago

@vargero if we manage to do a fix and follow your recommendations, how soon can this be merged?

I think it could be merged right away. I’ve been using a fork that contains my suggestion above for the past 4 months with no identifiable issues so far.

m-vdb commented 5 years ago

@vargero do you want to do the PR from your fork or do you want me to do a fresh PR? (I hit this roadblock today)

vargero commented 5 years ago

Whichever’s fine. Merging the current one would also work, the messages just won’t be as clear. In case you create a PR please tag me! Thanks

On Wed, 3 Apr 2019 at 08:51 Maxime Vdb notifications@github.com wrote:

@vargero https://github.com/vargero do you want to do the PR from your fork or do you want me to do a fresh PR? (I hit this roadblock today)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/segment-integrations/analytics-ios-integration-firebase/pull/37#issuecomment-479548941, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZv_C80kfD9i2gKbBzfQcBlGCSxxs8Dks5vdM4PgaJpZM4Yrc1I .

--

-- Bruno Leonardo Neves Machado Graduando de Sistemas de Informação CTC/UFSC

jawpio commented 5 years ago

Hey, When do you think the fix for this issue will be available (merged to master)? The issue combined with #33 is sort of blocker.

To fix #33 we need to configure the app on main_queue (which is done in AppDelegate), but then we hit this issue.

vargero commented 5 years ago

@jawpio it depends on whoever has merge access to this repository. Take a look at #42, it's the same fix but a little bit better in terms of logging. We're depending on @wcjohnson11 or @ladanazita to merge it.