search

talkjs / talkjs-flutter

Flutter SDK for the TalkJS Chat API
https://talkjs.com
BSD 3-Clause "New" or "Revised" License
9 stars 14 forks source link

Enhancements and Bug Fixes for Improved Compatibility and Functionality #35

Open arsarsars1 opened 1 year ago

arsarsars1 commented 1 year ago
bugnano commented 1 year ago

First of all thank you for your contribution. Before being able to accept any PR, no matter how small, we require every contributor to sign either the Individual CLA or the corporate CLA that you find in this repo, and email it to legal@talkjs.com

Having said that, there are a couple of things that I don't like in this PR:

  1. Please don't reformat the sources, as it makes it nearly impossible to understand what are actual changes, and what are only automated formatting changes
  2. We don't use Firebase for push notifications on iOS, so removing the flutter_apns dependency results in breaking the push notifications on iOS
arsarsars1 commented 1 year ago

Hi, thank you for your response and for reviewing the PR. I noticed that the APNS (Apple Push Notification Service) was disconnected from the Flutter code, which caused issues with iOS crash analytics. To resolve this, I removed the APNS integration. Thank you.



On 03-Jul-2023, at 4:54 PM, Franco Bugnano @.***> wrote:

First of all thank you for your contribution. Before being able to accept any PR, no matter how small, we require every contributor to sign either the Individual CLA or the corporate CLA that you find in this repo, and email it to @. @.> Having said that, there are a couple of things that I don't like in this PR:

Please don't reformat the sources, as it makes it nearly impossible to understand what are actual changes, and what are only automated formatting changes We don't use Firebase for push notifications on iOS, so removing the flutter_apns dependency results in breaking the push notifications on iOS — Reply to this email directly, view it on GitHub https://github.com/talkjs/talkjs-flutter/pull/35#issuecomment-1618078354, or unsubscribe https://github.com/notifications/unsubscribe-auth/AICXBKJEYPMCTR3YHZYH5WDXOKXHBANCNFSM6AAAAAAZ4IU5AE. You are receiving this because you authored the thread.

arsarsars1 commented 1 year ago

On 03-Jul-2023, at 4:59 PM, Abdul Rehman (A.R.S) @.***> wrote:

Hi, thank you for your response and for reviewing the PR. I noticed that the APNS (Apple Push Notification Service) was disconnected from the Flutter code, which caused issues with iOS crash analytics. To resolve this, I removed the APNS integration. Thank you.



On 03-Jul-2023, at 4:54 PM, Franco Bugnano @.***> wrote:

First of all thank you for your contribution. Before being able to accept any PR, no matter how small, we require every contributor to sign either the Individual CLA or the corporate CLA that you find in this repo, and email it to @. @.> Having said that, there are a couple of things that I don't like in this PR:

Please don't reformat the sources, as it makes it nearly impossible to understand what are actual changes, and what are only automated formatting changes We don't use Firebase for push notifications on iOS, so removing the flutter_apns dependency results in breaking the push notifications on iOS — Reply to this email directly, view it on GitHub https://github.com/talkjs/talkjs-flutter/pull/35#issuecomment-1618078354, or unsubscribe https://github.com/notifications/unsubscribe-auth/AICXBKJEYPMCTR3YHZYH5WDXOKXHBANCNFSM6AAAAAAZ4IU5AE. You are receiving this because you authored the thread.