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)
Tue, Oct 15, 8:50 PM
Unknown Object (File)
Tue, Oct 15, 8:50 PM
Unknown Object (File)
Sep 26 2024, 6:11 AM
Unknown Object (File)
Sep 16 2024, 1:59 PM
Unknown Object (File)
Sep 16 2024, 1:59 PM
Unknown Object (File)
Sep 16 2024, 1:59 PM
Unknown Object (File)
Sep 16 2024, 1:59 PM
Unknown Object (File)
Sep 12 2024, 8:10 PM
View All 55 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
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

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

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

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 34389

View Options

lib/selectors/user-selectors.js

Loading...