lib/reducers/integrity-reducer.js
76–81 ↗	(On Diff #37978)	Both replace and remove operations both support operating on several hashes at once by taking an array as an argument. However, in this reducer, I have the operation payload contain only one id to remove or one integrity thread hash to replace at a time. Do I have to worry about bundling these operations together if there are consecutive replace/remove operations?

will retitled this revision from [sqlite] [lib] Refactor integrity reducer to process threadStore ops with sqlite ops to [lib] Refactor integrity reducer to process threadStore ops with sqlite ops.Mar 11 2024, 3:11 PM

flow check. Not sure how to best test this right now. Might need to test later in diff stack

You could (only on your dev environment) leave the old logic above and result in state store in a variable, then compare it with the result state from ops and compare using _isEqual

lib/reducers/integrity-reducer.js
76–81 ↗	(On Diff #37978)	Unfortunately, I think we have to leave it as you implemented it because of ops ordering - but @michal could say more as he implemented this logic.

michal added inline comments.Mar 13 2024, 4:10 AM

lib/reducers/integrity-reducer.js
76–81 ↗	(On Diff #37978)	I think it's correct to bundle consecutive `replace` operations (and `remove` operations although I think these are less of a problem) if they aren't separated by another operation. And it would improve performance. If it's not too hard to implement, could we try and do it?

will planned changes to this revision.Mar 13 2024, 11:32 PM

will added a child revision: D11322: refactor integrity reducer starting state to use processStoreOps.Mar 13 2024, 11:37 PM

review feedback

will added inline comments.Mar 18 2024, 9:30 PM

lib/reducers/integrity-reducer.js
78 ↗	(On Diff #38152)	I wasn't able to find any specific examples in our codebase for grouping items like this. Let me know if this solution makes sense. Tested in future diffs and behavior looks correct
98–102 ↗	(On Diff #38152)	I didn't bundle our remove operations as we can already include several in a payload. Let me know if this also needs bundling.