refactor: replace trivial lodash functions with native equivalents #323
Conversation
|
How can we be sure this does not introduce bugs? |
|
How confident are you? :) |
As confident as I will get. Read through it thrice and did all the manual testing I could think of. Maybe we should maintain a manual test list for situations like this? |
|
I think you just have to ensure that it is well-tested. Perhaps by keeping a list of the places you have changed and then being sure that you test in a way that is likely to exercise those code paths. |
server/src/engine/queue.ts
Outdated
| waitingForPriorityQueue = {} | ||
| return values | ||
| } | ||
|
|
||
| let enqueueDebounced: () => void = makeEnqueueDebounced() | ||
|
|
||
| function handleEnqueue() { | ||
| if (_.isEmpty(waitingForPriorityQueue)) { | ||
| if (Object.keys(waitingForPriorityQueue).length === 0) { | ||
| return | ||
| } | ||
| priorityQueue.push(...emptyWaitingForPriorityQueue()) | ||
| startQueue() | ||
| } | ||
|
|
||
| function makeEnqueueDebounced() { |
There was a problem hiding this comment.
Perhaps this debouce can be in a separate PR?
There was a problem hiding this comment.
Then we'd have to keep in the dependency and make a third PR
There was a problem hiding this comment.
I think breaking this PR down to a few smaller PRs reduces the risk.
|
I don't remember exactly what I have tested when, so I can go through it one more time if you would prefer |
|
@magnus-madsen |
sockmaster27
changed the title
|
Until we have tests, I think I would only be comfortable if we did this slowly, one file at a time. That way we would minimize breakages and maximize the chance that we can catch all errors. |
|
Lets try for some tests. Then we can go file-by-file. I will close this meanwhile. (You can still find it). |
Builds on #322