search

dmanjunath / node-redshift

A simple collection of tools to help you get started with Amazon Redshift from node.js
69 stars 48 forks source link

Why are you unwrapping the Promise from pg? #13

Closed eudaimos closed 7 years ago

eudaimos commented 7 years ago

In your query code you are importing Bluebird simply to handle the returned promise, unwrap it, and then use a callback style.

Why?

vitaly-t commented 7 years ago

'Cos that's what people do when they do not understand how to use promises.

dmanjunath commented 7 years ago

@eudaimos @vitaly-t I personally don't like using promises, but that doesn't mean I don't want to stop others from using it- see https://github.com/dmanjunath/node-redshift/pull/11

vitaly-t commented 7 years ago

personally don't like using promises

This no longer bodes well for public libraries, ever since Node.js 7.6.0 that now supports async/await syntax by default, which works natively with promises, which is a hugely important feature.

dmanjunath commented 7 years ago

@vitaly-t I prefer async/await too but since it just got off --harmony, and not to mention node 7 is not a LTS release I don't want to write the query code using async/await without a transpiler. But if someone's using async/await the query function should still work as it is right now with full support for promises.

vitaly-t commented 7 years ago

I don't want to write the query code using async/await without a transpiler

This wasn't what I suggested. I simply pointed out that if you provide a promise-based interface, then anyone who already uses async/away can use such API natively, as async/await maps into promises directly.

ChrisCates commented 7 years ago

LOL I can't believe you guys are roasting @dmanjunath when I highly doubt y'all have used a Systems Level language before.

vitaly-t commented 7 years ago

Roasting? No worries, we are flipping sides :smile:

dmanjunath commented 7 years ago

@ChrisCates thanks for the backup haha. But this is getting off topic very quickly. Unless there's a concrete issue or code improvement I'm closing this issue.