Ok, that makes sense. But what do you think about using filteredMessageStore as an argument of processMessageStoreOperations? By doing that it should be ok to just use the result of processMessageStoreOperations - it will be more maintainable

const processedMessageStore = processMessageStoreOperations(
  filteredMessageStore,
  messageStoreOperations,
);

return {
  messageStoreOperations,
  messageStore: processedMessageStore,

That definitely makes sense, but we can't do it just yet: https://phab.comm.dev/D4201?vs=on&id=13644#121744. It'll take some more refactoring, but that's definitely the goal. These initial diffs are mostly about removing usages of assertMessageStoreMessagesAreEqual(...) so it can be removed altogether.

Slight feedback to @atul – I think if you find yourself doing a refactor where you do "the same thing" over and over in different places, it can actually be easier to review if you just put it all into one slightly larger diff

In D4202#122025, @ashoat wrote:

Slight feedback to @atul – I think if you find yourself doing a refactor where you do "the same thing" over and over in different places, it can actually be easier to review if you just put it all into one slightly larger diff

Yeah I agree, doing it like this was was tedious for me + reviewers. Have a diff locally where I just do this for all remaining actions in one go.