Refactor integrity reducer to use sqlite ops
Depends on D11287
Details
Details
flow check. Not sure how to best test this right now. Might need to test later in diff stack
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| 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? |
Comment Actions
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. |
| 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? |
| lib/reducers/integrity-reducer.js | ||
|---|---|---|
| 78 | 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 | I didn't bundle our remove operations as we can already include several in a payload. Let me know if this also needs bundling. | |