Page MenuHomePhabricator

Move android rescinds and badge updates to lib
AcceptedPublic

Authored by marcin on Thu, Jun 13, 8:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 22, 4:45 AM
Unknown Object (File)
Fri, Jun 21, 6:43 AM
Unknown Object (File)
Fri, Jun 21, 6:42 AM
Unknown Object (File)
Thu, Jun 20, 11:06 PM
Unknown Object (File)
Thu, Jun 20, 10:28 PM
Unknown Object (File)
Wed, Jun 19, 3:47 PM
Unknown Object (File)
Tue, Jun 18, 3:45 AM
Unknown Object (File)
Tue, Jun 18, 3:45 AM
Subscribers

Details

Reviewers
tomek
kamil
Summary

This differential move Android rescinds and badge updates to lib and makes the keyserver use shared code.

Test Plan
  1. Flow
  2. Test that rescind and badge updates work on Android

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

I strongly encourage you to rethink the amount of type spreads you're doing, especially when they're nested within other types

lib/push/notif-creators.js
329

In general, I am skeptical of this approach of nesting unions inside a spread. In the past I've seen Flow not handle it well. I usually opt to make the upper type a union, and to include the shared properties in each branch (senderDeviceID, platformDetails) via a spread

333
lib/types/notif-types.js
140

What is this spread doing?

165

What is this spread doing?

lib/push/android-notif-creators.js
266 ↗(On Diff #41556)

This is logic copy pasted from keyserver/push/rescind.js but extended to allow rescinds without notification id (thick thread rescinds).

337 ↗(On Diff #41556)

This logic is copy-pasted from keyserver/src/send.js but extended to allow thick thread badge updates.

You forgot to address @ashoat's comments

lib/types/notif-types.js
168–174 ↗(On Diff #41556)

This applies to types above

This revision is now accepted and ready to land.Tue, Jun 25, 3:26 AM