Page MenuHomePhabricator

[lib] Merge consecutive membership robotext messages
ClosedPublic

Authored by ashoat on Oct 9 2024, 8:35 PM.
Tags
None
Referenced Files
F3339187: D13689.diff
Thu, Nov 21, 6:57 PM
Unknown Object (File)
Wed, Nov 20, 9:21 AM
Unknown Object (File)
Sun, Nov 17, 10:20 AM
Unknown Object (File)
Wed, Nov 13, 6:40 AM
Unknown Object (File)
Wed, Nov 13, 6:35 AM
Unknown Object (File)
Tue, Nov 12, 11:34 PM
Unknown Object (File)
Tue, Nov 12, 11:32 PM
Unknown Object (File)
Sun, Nov 10, 6:06 PM
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.