Paths

Table of Contentst

Differential D12770

[Tunnelbroker][lib] add FCM notif message type
ClosedPublic
Actions

Authored by kamil on Jul 16 2024, 1:51 AM.

Tags

None

Referenced Files

	F3359470: D12770.diff
	Sun, Nov 24, 9:22 AM

	Unknown Object (File)
	Tue, Nov 12, 12:57 AM

	Unknown Object (File)
	Sat, Nov 9, 8:55 AM

	Unknown Object (File)
	Sat, Nov 9, 8:43 AM

	Unknown Object (File)
	Sat, Nov 2, 2:16 AM

	Unknown Object (File)
	Fri, Nov 1, 10:19 AM

	Unknown Object (File)
	Fri, Nov 1, 10:19 AM

	Unknown Object (File)
	Fri, Nov 1, 10:18 AM

View All 48 Files

Subscribers

Details

Reviewers

bartek
marcin

Commits

rCOMMf614a5cf3713: [Tunnelbroker][lib] add FCM notif message type

Summary

Message send from client to send notif using FCM service.

Depends on D12769

Test Plan

Tested in D12772.

Diff Detail

Repository

Branch

land-fcm-3

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.Jul 16 2024, 1:51 AM

kamil held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 16 2024, 1:51 AM

kamil added a child revision: D12771: [Tunnelbroker] implement sending FCM notifs by client.Jul 16 2024, 1:53 AM

Harbormaster completed remote builds in B30365: Diff 42327.Jul 16 2024, 2:07 AM

kamil published this revision for review.Jul 16 2024, 2:45 AM

kamil added inline comments.

lib/types/tunnelbroker/notif-types.js
16 ↗	(On Diff #42327)	This could take only `HIGH` and `NORMAL` values but wanted to make it in-sync with Rust types

kamil edited the test plan for this revision. (Show Details)Jul 16 2024, 2:48 AM

ashoat added inline comments.Jul 16 2024, 6:42 AM

lib/types/tunnelbroker/notif-types.js
16 ↗	(On Diff #42327)	I'd rather make it more specific in JS, if Rust doesn't support the specificity

bartek added inline comments.Jul 18 2024, 7:25 AM

lib/types/tunnelbroker/notif-types.js
16 ↗	(On Diff #42327)	In Rust it's technically a string, but parsing will fail (or at least ignore, in D12767 it's set to `None`) if it's not one of the above, so I'd use `+priority: 'NORMAL' \| 'HIGH'.` here

kamil added inline comments.Jul 18 2024, 8:18 AM

lib/types/tunnelbroker/notif-types.js
16 ↗	(On Diff #42327)	makes sense, I'll update this to `+priority: 'NORMAL' \| 'HIGH'.` before landing

improve type

bartek accepted this revision.Jul 19 2024, 5:38 AM

This revision is now accepted and ready to land.Jul 19 2024, 5:38 AM

marcin accepted this revision.Jul 19 2024, 5:40 AM

This revision was landed with ongoing or failed builds.Jul 19 2024, 5:44 AM

Closed by commit rCOMMf614a5cf3713: [Tunnelbroker][lib] add FCM notif message type (authored by kamil). · Explain Why

This revision was automatically updated to reflect the committed changes.

kamil added a commit: rCOMMf614a5cf3713: [Tunnelbroker][lib] add FCM notif message type.

Harbormaster completed remote builds in B30508: Diff 42530.Jul 19 2024, 5:48 AM

Revision Contents
Changeset List

Path

Size

lib/

types/

tunnelbroker/

8 lines

shared/

tunnelbroker_messages/

src/

messages/

1 line

15 lines

Diff 42530

lib/types/tunnelbroker/notif-types.js

Loading...

shared/tunnelbroker_messages/src/messages/mod.rs

Loading...

shared/tunnelbroker_messages/src/messages/notif.rs

Loading...