create all docs before calling back #424
Conversation
| docs.forEach(function(doc) { | ||
| collection.update(doc, doc, {upsert: true}, function (err, created) { | ||
| firstErr = firstErr || err; | ||
| totalCreated += created; |
There was a problem hiding this comment.
This is especially sketchy, as created is an object, but the initial value of totalCreated is a number, turning totalCreated into a string of the form 0{"ok":1,"nModified":1,"n":1}{"ok":1,"nModified":1,"n":1}...
|
Such a fan of this, I created PR, is this good to go or does it need to wait for some reason? |
|
It's basically good to go from my perspective. One could argue that tests 3 and 2 should be swapped (since test 3 checks for more docs, it's more likely to catch a regression). One could also nitpick that |
|
Just pushed a commit with precisely those changes. |
dduugg
force-pushed
the
wip/fix-create
branch
2 times, most recently
from
1ba55a0
to
cfe1e51
Compare
1 similar comment
|
Been out of town, want to a do a quick test and merge this tonight or tomorrow. |
Resolves nightscout#423. Also changes the callback signature, invoking it with the error and docs, rather than the error, totalCreated, and docs. The third paramter isn't used, and the second parameter should be an array. (Since totalCreated was 0 on callback, it was falsy and replaced with an empty array when used in format_entries.)
flagged by webstorm: "return is unnecessary as the last statement in a function with no return value"
1 similar comment
|
Tested, thanks @dduugg |
create all docs before calling back
This resolves an issue where the
createfunction inentries.jswasn't waiting for the updates to complete before calling back. This would frequently cause Travis failures, when tests would expect entries that had yet to be completed.resolves #423