Convert logic to ops approach.
Depends on D10759
Details
Details
- Refresh website on web (it cause sessionChange with defined cookie).
- Make sure state changes the same way as before refactor.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- land-ks-store
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
It's not good that my review was necessary here. Both @kamil and @inka should think about how this issue was missed. We need to be more careful and thoughtful when submitting and reviewing code. I should not have to be a phantom reviewer for every diff in the codebase, but it often feels like if I wasn't here, things would get missed...
| lib/reducers/keyserver-reducer.js | ||
|---|---|---|
| 129 ↗ | (On Diff #35915) | In the old logic, the second replacement would keep the updated cookie from the first replacement. After your changes, it looks like the updated cookie is discarded, since the second`replace_keyserver` pulls from the non-updated state version (where previously it was updated). I think we should avoid queueing up two replace_keyservers for the same keyserver. Can you coalesce the changes into a single op? |
| lib/reducers/keyserver-reducer.js | ||
|---|---|---|
| 134 ↗ | (On Diff #36216) | I actually think it's worth skipping the op, but _isEqual seems like overkill. Maybe just make a variable let keyserverUpdated = false and set it in the two conditions? |