Add functionality to copy feed and append to it.#38
Add functionality to copy feed and append to it.#38benjaminettori wants to merge 3 commits intoCodeForPhilly:masterfrom
Conversation
|
@flyingzumwalt It seems node did not start supporting promises natively until v 0.11, which means the build for v 0.10 fails. Is this an issue? |
Change module export name. Squash commits for appending feed
|
@benjaminettori I think it's fine for us to only support node 0.11 and higher. Remove 0.10 from the travis.yml and update the Readme to call out the dependency on node >= 0.11. |
| } | ||
| }) | ||
| }) | ||
| } |
There was a problem hiding this comment.
this might append blocks in the wrong order as this is async. you probably wanna do this
loop(null)
function loop (err) {
if (err) return reject(err)
refFeed.get(newFeed.blocks, function (err, block) {
if (err) return cb(err)
if (!block) return resolve() // done
newFeed.append(block, loop)
})
}|
@mafintosh you commented in irc that you're not a fan of promises for server-side code. Could you elaborate? I think @benjaminettori chose to use promises because it was a clearer way to handle nested callbacks. @benjaminettori could you provide an example of a place where the code would get confusing without promises? |
|
@flyingzumwalt Actually I may have a way to do this without promises, I will submit another PR if I finalize it. I was using promises because I wanted to return a feed object, but this is probably not the best way to approach the issue. Thanks @mafintosh for catching that mistake and for the clever code snippet. |
…Add tape tests for these methods.
|
@flyingzumwalt I refactored the code and added methods to replicate and append to feeds without using promises. I also added some tests around these new methods. |
No description provided.