search

CodeForPhilly / jawn

'Git for Tabular Data'
http://datjawn.com
BSD 3-Clause "New" or "Revised" License
44 stars 9 forks source link

Add functionality to copy feed and append to it. #38

Open benjaminettori opened 8 years ago

benjaminettori commented 8 years ago

@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?

flyingzumwalt commented 8 years ago

@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.

flyingzumwalt commented 8 years ago

@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?

benjaminettori commented 8 years ago

@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.

benjaminettori commented 8 years ago

@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.