heroku / heroku-connect-plugin

CLI plugin for Heroku Connect (experimental)
Apache License 2.0
23 stars 8 forks source link

Add a command to retrieve the state of a mapping #21

Closed gulopine closed 8 years ago

iamjem commented 8 years ago

Same comment as other PR, sssuming the generator stuff is safe to use, this looks :+1:

I must say, this helps make node's async spaghetti more synchronous-looking. I'll have to read more about how errors are handled with the native spec, co's error handling looks simple enough.

gulopine commented 8 years ago

The nice thing about error handling between co and cli.command is that any time we reject a promise and don't explicitly do anything else with it, it'll automatically get nicely formatted output that's consistent with the rest of the toolbelt. So they get the little red arrow and an indent before the actual message.

There are some cases where we need to handle errors specifically, and so far, I've find that easier to do that with the traditional pair of .then and .catch, but I'm probably just not familiar enough with co error handling to do it "better".

That said, I haven't managed to get generators to work inside of a promise for some reason, so there are other cases where I'm still using .then when I'd otherwise use a yield. Roughly, this is all a first draft, and I'm sure we can improve it over time. It's already a big win, though. :grinning: