Details

Reviewers

atul
tomek
ashoat

Commits

rCOMM6ac8b92473c6: Add NotificationsBridgeQueue module

Summary

This differential defines and implements NotificationsBridgeQueue module that will be used in CommIOSNotifications implementation to store JS-related events and actions before JS thread is ready. Code is copy-pasted from Ashoat's RNNotifications fork.
Ashoat's fork: https://github.com/Ashoat/react-native-notifications

Test Plan

Build the app.

Diff Detail

Repository

rCOMM Comm

Branch

marcin/eng-2334

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Dec 28 2022, 3:11 AM

Herald added a subscriber: ashoat. · View Herald TranscriptDec 28 2022, 3:11 AM

Rebase

Harbormaster completed remote builds in B14819: Diff 20210.Dec 28 2022, 3:25 AM

Harbormaster completed remote builds in B14822: Diff 20213.Dec 28 2022, 3:29 AM

marcin requested review of this revision.Dec 28 2022, 3:29 AM

marcin added a parent revision: D6064: CommIOSNotifications module functionality definition.Dec 28 2022, 3:43 AM

marcin added a child revision: D6066: Start CommIOSNotifications implementation by adding JavaScript events handlers..Dec 28 2022, 4:44 AM

It might be a good idea to add a link to the fork in the summary.

This revision is now accepted and ready to land.Dec 29 2022, 6:03 AM

marcin edited the summary of this revision. (Show Details)Dec 30 2022, 4:17 AM

Copy pasted code will be refactored.

Refactor CommIOSNotificationsBridgeQueue module. Not react-native-notifications fork copy paste any more.

This revision is now accepted and ready to land.Jan 9 2023, 7:04 AM

marcin retitled this revision from Add NotificationsBridgeQueue module - copy pasted from RNNotifications Ashoat fork to Add NotificationsBridgeQueue module.Jan 9 2023, 7:05 AM

Harbormaster completed remote builds in B15174: Diff 20686.Jan 9 2023, 7:18 AM

Please make sure you've rewritten every single line in your style. It's concerning seeing two styles of conditionals in this diff (seems like an artifact of copy-paste). Conditionals with curly braces are something we generally avoid

native/ios/Comm/CommIOSNotifications/CommIOSNotificationsBridgeQueue.mm
26–27 ↗	(On Diff #20686)	Let's avoid this style – please put the curly braces back, like below
42–43 ↗	(On Diff #20686)	Here too

ashoat requested changes to this revision.Jan 9 2023, 9:52 AM

This revision now requires changes to proceed.Jan 9 2023, 9:52 AM

Remove actions queue and relevant methods. They were not used in our code.

ashoat accepted this revision.Jan 17 2023, 7:13 AM

This revision is now accepted and ready to land.Jan 17 2023, 7:13 AM

Harbormaster completed remote builds in B15403: Diff 21005.Jan 17 2023, 7:24 AM

marcin updated this revision to Diff 21134.Jan 20 2023, 2:26 AM

This comment was removed by marcin.

Remove methods related to actions since they are not used anywhere in the code. I introduced this change earlier this week, but forgot to update phabricator

Harbormaster completed remote builds in B15491: Diff 21134.Jan 20 2023, 2:39 AM

Harbormaster completed remote builds in B15492: Diff 21135.Jan 20 2023, 2:49 AM

Rebase before landing

Harbormaster completed remote builds in B15707: Diff 21416.Jan 27 2023, 1:18 AM

Closed by commit rCOMM6ac8b92473c6: Add NotificationsBridgeQueue module (authored by marcin). · Explain WhyJan 27 2023, 1:52 AM

This revision was automatically updated to reflect the committed changes.

marcin added a commit: rCOMM6ac8b92473c6: Add NotificationsBridgeQueue module.