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
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.