-
Notifications
You must be signed in to change notification settings - Fork 985
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
Remove firebase #9308
Remove firebase #9308
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (63)
|
installation-name (get-in cofx [:db :pairing/installations installation-id :name]) | ||
device-type utils.platform/os] | ||
(protocol/send (transport.pairing/PairInstallation. installation-id device-type installation-name fcm-token) nil cofx))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cammellos what needs to be done on go side to remove fcm tokens and device infos? I think there is also some firebase stuff on go side?
@yenda ofc. |
@churik I tested all the steps described in the summary on both devices. |
src/status_im/contact/core.cljs
Outdated
@@ -4,7 +4,7 @@ | |||
[status-im.multiaccounts.model :as multiaccounts.model] | |||
[status-im.transport.filters.core :as transport.filters] | |||
[status-im.contact.db :as contact.db] | |||
[status-im.contact.device-info :as device-info] | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I naively though that cljfmt would remove them
src/status_im/pairing/core.cljs
Outdated
@@ -4,7 +4,7 @@ | |||
[status-im.chat.models :as models.chat] | |||
[status-im.contact.core :as contact] | |||
[status-im.contact.db :as contact.db] | |||
[status-im.contact.device-info :as device-info] | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra line?
27% of end-end tests have passed
Failed tests (36)Click to expand
Passed tests (13)Click to expand
|
100% of end-end tests have passed
Passed tests (49)Click to expand |
@churik do you want to run some additional manual tests before this is merged? |
Tested on IOS 13.0 and Android 8.0:
Additionally checked:
Ready to go from my side. |
@rachelhamlin we can then in the worst case simply revert the commit. But reintroducing Firebase would be a bad decision. |
76ebba3
to
1edd3de
Compare
1edd3de
to
1940ac5
Compare
Signed-off-by: yenda <eric@status.im>
1940ac5
to
7bb45fd
Compare
fix #9179
fix #9136 (no notification no problem)
fix #8742 (firebase removed)
fix #8555 notifications removed
fix #8554 no more notifications
fix #8550 fix #8217 fix #8216
fix #7438 ios doesn't deserve notifications anyway
Remove Firebase, Google Services and all the Google ads crap from the project.
Remove fcm-token and device infos
Remove push-notifications
Test protocol
What could go wrong: