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

Feat/v4 #108

Closed
wants to merge 12 commits into from
Closed

Feat/v4 #108

wants to merge 12 commits into from

Conversation

fabriziomoscon
Copy link
Collaborator

No description provided.

@fabriziomoscon fabriziomoscon force-pushed the feat/v4 branch 2 times, most recently from 5c00a04 to 3e18619 Compare January 8, 2019 19:46
@fabriziomoscon fabriziomoscon force-pushed the feat/v4 branch 2 times, most recently from b5424fc to 20f9cdb Compare December 4, 2019 21:15
@fabriziomoscon
Copy link
Collaborator Author

fabriziomoscon commented Dec 4, 2019

@Pagebakers @gitstud my intention is to release breaking changes with this branch and on top of it add #130
any help in testing this would br greatly appreciated.

@jdegger
Copy link
Collaborator

jdegger commented Dec 5, 2019

@fabriziomoscon when you have a working fork (with or without breaking changes) which works for both iOS and Android, we are willing to immediately test it throughougly for you. Please ping us when we are needed!

(cc: @wesdewitte)

@fabriziomoscon
Copy link
Collaborator Author

fabriziomoscon commented Dec 5, 2019

@jdegger I have been testing this branch for a long time and it works on real device for real calls, but it doesn't use Twilio's latest library, because I didn't notice there were going to be deprecations. The code in #130 should be added on top of it to test more, then we will need to add the new iOS twilio SDK and integrate changes

@jdegger
Copy link
Collaborator

jdegger commented Dec 5, 2019

@fabriziomoscon Sounds good. Your work is aimed at SDK4, however SDK5 is already been released. Would this be an opportunity to immediately go for V5?

At the moment we do not have the development power to actually help with the PR, however let me know if it needs any testing or if you need help on any other aspect.

@Pagebakers
Copy link
Contributor

Great to see some action here :)

I'll have a look if I can merge this next week.

@fabriziomoscon
Copy link
Collaborator Author

fabriziomoscon commented Dec 5, 2019

@jdegger this branch is called v4 for RNtwilio breaking changes from v3, when I started the work there weren't SDK above V2 from twilio.
I need to look better into #130 to fully answer your question.

@fabriziomoscon
Copy link
Collaborator Author

Correction: Yesterday I mistaken referenced issue 127 when I meant PR 130 - changed my comments.

@gitstud
Copy link
Contributor

gitstud commented Dec 5, 2019

Commits are clean! I will try to test this out on iOS this weekend

@aniravi24
Copy link
Contributor

aniravi24 commented Dec 10, 2019

Is this PR removing the firebase service on Android that shows notifications for your incoming calls->ongoing calls->missed calls? Or does that still work?

@fabriziomoscon
Copy link
Collaborator Author

@aniravi24 if that is the case it was not done on purpose.
implementation 'com.google.firebase:firebase-messaging:17.+' is still there.
I guess I will need to test more for this specific case on Android.

@aniravi24
Copy link
Contributor

Got it. I saw it was removed from AndroidManifest.xml so that's why I thought it was being removed.

@gitstud
Copy link
Contributor

gitstud commented Dec 14, 2019

I didn't see my logs in deviceReady or deviceNotReady, I was able to make an outbound call and hangup

@lonnylot lonnylot mentioned this pull request Dec 20, 2019
@aniravi24
Copy link
Contributor

aniravi24 commented Dec 24, 2019

Working fine on Android for us! Ignore my earlier comment above, I misread the changes being made. We'll give iOS a test soon too (I assume it'll work fine given that the only thing that seems to have changed is renaming of some properties on the iOS side).

@aniravi24
Copy link
Contributor

Looks good.

@fabriziomoscon
Copy link
Collaborator Author

these commits are now included into #144

@fabriziomoscon fabriziomoscon deleted the feat/v4 branch January 18, 2020 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants