Page MenuHomePhabricator
Differential D12371

[lib] move processing `MESSAGE_TO_DEVICE` to a component
ClosedPublic
Actions

Authored by kamil on Mon, Jun 10, 4:10 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jun 26, 8:49 AM
Unknown Object (File)
Tue, Jun 25, 2:35 PM
Unknown Object (File)
Mon, Jun 24, 9:48 PM
Unknown Object (File)
Sun, Jun 23, 11:41 PM
Unknown Object (File)
Sat, Jun 22, 5:24 AM
Unknown Object (File)
Fri, Jun 21, 5:51 AM
Unknown Object (File)
Fri, Jun 21, 5:51 AM
Unknown Object (File)
Thu, Jun 20, 7:17 PM
View All 24 Files
Subscribers
ashoat

Details

Reviewers
bartek
tomek
Commits
rCOMMc5dc4cd1873a: [lib] move processing `MESSAGE_TO_DEVICE` to a component
Summary
  1. Cleans messy TunnelbrokerContext
  2. Makes it possible to convert peerToPeerMessageHandler to a hook and avoids passing a lot of callbacks as arguments
  3. Makes it possible to call sendMessage from peerToPeerMessageHandler (needed to implement ENG-7837).
Test Plan

Flow - this is just moving code to a child component. Functionality tested later in the stack.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil created this revision.Mon, Jun 10, 4:10 AM
kamil held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptMon, Jun 10, 4:10 AM
kamil added a child revision: D12372: [lib] convert `peerToPeerMessageHandler` to a hook.Mon, Jun 10, 4:19 AM
Harbormaster completed remote builds in B29564: Diff 41151.Mon, Jun 10, 4:26 AM
kamil edited the summary of this revision. (Show Details)Mon, Jun 10, 9:38 AM
kamil added inline comments.
lib/tunnelbroker/tunnelbroker-context.js
395–404 ↗(On Diff #41151)

not sure about best practices but this feels the best for this particular use case (passing ref as prop and reading it from promise later)

kamil published this revision for review.Mon, Jun 10, 9:50 AM
ashoat added inline comments.Mon, Jun 10, 1:32 PM
lib/tunnelbroker/tunnelbroker-context.js
422 ↗(On Diff #41151)

Unusual pattern to use "if" in a boolean variable name

How about doesSocketExist?

tomek accepted this revision.Wed, Jun 12, 4:25 AM
This revision is now accepted and ready to land.Wed, Jun 12, 4:25 AM
bartek accepted this revision.Wed, Jun 12, 8:41 AM
kamil updated this revision to Diff 41284.Thu, Jun 13, 7:43 AM

ifSocketExists -> doesSocketExist

Harbormaster completed remote builds in B29660: Diff 41284.Thu, Jun 13, 8:00 AM
Closed by commit rCOMMc5dc4cd1873a: [lib] move processing `MESSAGE_TO_DEVICE` to a component (authored by kamil). · Explain WhyThu, Jun 13, 9:08 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMMc5dc4cd1873a: [lib] move processing `MESSAGE_TO_DEVICE` to a component.

Revision Contents
Changeset List

PathSize
lib/
tunnelbroker/
111 lines
87 lines

Diff 41295

View Options

lib/tunnelbroker/peer-to-peer-message-handler.js

Loading...
View Options

lib/tunnelbroker/tunnelbroker-context.js

Loading...