search

rethinkdb / rethinkdb-example-nodejs

137 stars 98 forks source link

rewrote the express example using the 2.0.0 driver #7

Closed Prinzhorn closed 9 years ago

Prinzhorn commented 9 years ago

See #6

I left the angular code untouched, except for updating the URLs and fixing space/quote inconsistencies.

The app.js is basically completely rewritten to use the new driver (and a single connection), correctly handle errors and use the async module for readability.

This is the first time I have ever used RethinkDB, so please comment on all the things. I don't know if I've followed best practices etc.

After we're happy with it, we need someone for the koa and promises rewrite as I don't have any experience with them and don't know about best practices.

Ping @deontologician and @robertklep for review

Prinzhorn commented 9 years ago

Any thoughts on code style? I went with my personal preference (tabs) since the project was inconsistent with tabs and four spaces. I also went with single quotes as the project had mixed single and double quotes.

robertklep commented 9 years ago

LGTM :+1: (also tested it locally, it's running fine)

I don't care much for tabs (2-space indents #ftw), but mixing is bad so either tabs or spaces for consistency. Same for quotes.

deontologician commented 9 years ago

Looks good, I really like async.waterfall, that's a nice library.

Other than the issue I noted about clearing completed items, it looks good. I also agree with @robertklep that 2 spaces is better than tabs, but I wouldn't not accept the PR over that

Prinzhorn commented 9 years ago

I'll look into the issue and change the indentation later today.

Prinzhorn commented 9 years ago

2 spaces, single quotes and fixed the "clear completed" thing.

deontologician commented 9 years ago

Looks good. Thanks a lot @Prinzhorn