Rename deviceIDTypes to deviceTypes, add comments
ClosedPublic
Actions

Authored by inka on Oct 5 2022, 5:57 AM.

Details

Reviewers

tomek
michal
kamil
bartek
• jon

Commits

rCOMM1482f5e7c164: Rename deviceIDTypes to deviceTypes, add comments

Summary

As suggested in comments on D4754, deviceIDTypes has been renamed to deviceTypes. The comments are needed because we have a logical dependency between these functions and variables that are not enforced by the code.

Test Plan

See with Redux DevTools that device ID is still present in Redux store on web

Diff Detail

Repository

rCOMM Comm

Branch

inka/deviceID_rs_rebuild

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.Oct 5 2022, 5:57 AM

Herald added a reviewer: • jon. · View Herald TranscriptOct 5 2022, 5:57 AM

Herald added subscribers: • abosh, atul, ashoat. · View Herald Transcript

inka added a parent revision: D4754: [native] Generate device ID using Rust.Oct 5 2022, 5:58 AM

inka added a parent revision: D4868: [web] Add device ID to redux.

inka removed a parent revision: D4868: [web] Add device ID to redux.

inka added a parent revision: D4887: [web] Add presisting device ID.

inka retitled this revision from Rename deviceIDTypes to deviceTypes to Rename deviceIDTypes to deviceTypes, add comments.Oct 5 2022, 6:03 AM

inka edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B12627: Diff 17355.Oct 5 2022, 6:08 AM

inka requested review of this revision.Oct 5 2022, 6:08 AM

defer to someone else for js

services/tunnelbroker/src/Constants.h
51–54	We should eventually consolidate shared logic, and avoid it getting out-of-sync.

tomek accepted this revision.Oct 6 2022, 1:41 AM

This revision is now accepted and ready to land.Oct 6 2022, 1:41 AM

@jon I didn't add you, Herald did. Also: we (me, @tomek and @ashoat) talked about what should be done about this keeping in sync, but because all these are in different languages, and we don't expect this to change much, it has been decided that for now it would be too much work to have this be synced automatically.