Page MenuHomePhabricator
Differential D10233

[lib] Simplify `getRelativeMemberInfos`
ClosedPublic
Actions

Authored by atul on Dec 7 2023, 11:40 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 5 2024, 3:15 PM
Unknown Object (File)
Mar 5 2024, 3:15 PM
Unknown Object (File)
Mar 5 2024, 3:15 PM
Unknown Object (File)
Mar 5 2024, 3:15 PM
Unknown Object (File)
Mar 5 2024, 3:15 PM
Unknown Object (File)
Jan 28 2024, 2:33 AM
Unknown Object (File)
Dec 29 2023, 2:56 AM
Unknown Object (File)
Dec 28 2023, 9:03 PM
View All 16 Files
Subscribers
None

Details

Reviewers
ashoat
ginsu
tomek
rohan
Commits
rCOMM1b12b933f5cc: [lib] Simplify `getRelativeMemberInfos`
Summary

In subsequent diff going to split into two separate functions: getLegacyRelativeMemberInfos and getMinimallyEncodedRelativeMemberInfos and then toggle between then based on whether input RawThreadInfo is minimallyEncoded or not.

Since that diff is about duplicating existing functionality and branching... figure changing the actual functionality may be annoying for reviewers even though it's minimal (?)

Depends on D10227

Test Plan

CI/flow/etc.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.Dec 7 2023, 11:40 AM
atul added a child revision: D10235: [lib] Update `getRelativeMemberInfos` to handle both `MinimallyEncoded` and `Legacy` variants.Dec 7 2023, 11:53 AM
Harbormaster completed remote builds in B24865: Diff 34389.Dec 7 2023, 12:25 PM
atul requested review of this revision.Dec 7 2023, 12:25 PM
ashoat accepted this revision.Dec 8 2023, 2:52 PM
ashoat added inline comments.
lib/selectors/user-selectors.js
74–86 ↗(On Diff #34389)

With the new Flow, I wonder if you can get rid of the branching

This revision is now accepted and ready to land.Dec 8 2023, 2:52 PM
atul updated this revision to Diff 34589.Dec 13 2023, 1:43 PM

land

This revision was landed with ongoing or failed builds.Dec 13 2023, 1:45 PM
Closed by commit rCOMM1b12b933f5cc: [lib] Simplify `getRelativeMemberInfos` (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM1b12b933f5cc: [lib] Simplify `getRelativeMemberInfos`.
Harbormaster completed remote builds in B25034: Diff 34589.Dec 13 2023, 2:21 PM
atul added inline comments.Dec 13 2023, 2:39 PM
lib/selectors/user-selectors.js
74–86 ↗(On Diff #34389)

I was about to do that, but noticed that in the currentUserID case we unshift whereas in the non-currentUserID case we push.

Don't have full context on why we need these ordered in any particular way, but didn't want to mess with existing assumptions here.

Definitely let me know if I'm missing something and I'm happy to address.

Revision Contents
Changeset List

PathSize
lib/
selectors/
10 lines

Diff 34594

View Options

lib/selectors/user-selectors.js

Loading...