Page MenuHomePhabricator

[lib] Merge consecutive membership robotext messages
ClosedPublic

Authored by ashoat on Oct 9 2024, 8:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 5:58 AM
Unknown Object (File)
Mon, Nov 25, 3:13 AM
Unknown Object (File)
Nov 22 2024, 6:37 AM
Unknown Object (File)
Nov 22 2024, 1:44 AM
Unknown Object (File)
Nov 22 2024, 1:23 AM
Unknown Object (File)
Nov 21 2024, 6:57 PM
Unknown Object (File)
Nov 20 2024, 9:21 AM
Unknown Object (File)
Nov 17 2024, 10:20 AM
Subscribers
None

Details

Summary

This reduces a bunch of noise in community roots when people get auto-added to the communities.

Depends on D13688

Test Plan

Diff Detail

Repository
rCOMM Comm
Branch
ashoat/plural_join_thread
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Note that the extra vertical spacing in the test video is caused by ENG-2928, and is unrelated to the work here

tomek added inline comments.
lib/shared/messages/join-thread-message-spec.js
158–170 ↗(On Diff #45048)

All but one generated robotext messages will be discarded while merging the messages, which means that we're performing O(n^2) operations where n is a number of consecutive join thread messages. This isn't a big issue unless we expect a huge number of thread joins.

lib/shared/messages/remove-members-message-spec.js
39 ↗(On Diff #45048)
This revision is now accepted and ready to land.Oct 11 2024, 7:58 AM

Use ^ as separator instead of |

Undo last update and address feedback

lib/shared/messages/join-thread-message-spec.js
158–170 ↗(On Diff #45048)

Makes sense. I don't think it should be a big concern, and I didn't notice any performance issues while testing

This revision was landed with ongoing or failed builds.Oct 13 2024, 6:44 PM
This revision was automatically updated to reflect the committed changes.