search

1602 / jugglingdb

Multi-database ORM for nodejs: redis, mongodb, mysql, sqlite3, postgresql, arango, in-memory...
http://1602.github.io/jugglingdb/
2.04k stars 242 forks source link

added support for update queries #301

Closed pasindud closed 11 years ago

dgsan commented 11 years ago

Note related pull request for the MySQL adapter: https://github.com/jugglingdb/mysql-adapter/pull/44

The concept behind these pull requests doesn't seem bad, but they look to need a little code de-duplication outside of their lack of unit tests.

pasindud commented 11 years ago

the update implementation can be done in adapter or JDB core wht method is good?

anatoliychakkaev commented 11 years ago

Implementation can be done in adapter, interface should be declared in core.

On Wed, Jul 10, 2013 at 7:55 AM, Pasindu De Silva notifications@github.comwrote:

the update implementation can be done in adapter or JDB core wht method is good?

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/pull/301#issuecomment-20720319 .

dgsan commented 11 years ago

If it's still connecting should it return rather than defer until connected event?

Related, I think the basic sql tests you put in the mysql-adapter should maybe end up in core's tests so they can be shared.

anatoliychakkaev commented 11 years ago

@dgsan as usual, it should wait for 'connected' event if it's still in 'connecting' state.

On Wed, Jul 10, 2013 at 11:21 PM, dgsan notifications@github.com wrote:

If it's still connecting should it return rather than defer until connected event?

Related, I think the basic sql tests you put in the mysql-adapter should maybe end up in core's tests so they can be shared.

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/pull/301#issuecomment-20766286 .

Thanks, Anatoliy Chakkaev

dgsan commented 11 years ago

Yeah, I didn't realize what stillConnecting() does. It's a nifty trick.

pasindud commented 11 years ago

how to write the update tests in JDB core , i tried it didnt work

dgsan commented 11 years ago

You should just check that the work when running with mysql adapter. Someone else will probably have to implement matching functionality for memory adapter, etc., that are used when running JDB tests without an adapter, so running the core tests from jdb itself would probably be expected to fail at this point.

pasindud commented 11 years ago

is they anymore to be done in this ?

1602 commented 11 years ago

@pasindud use four spaces for identation and only use identation where it's necessary, everythings else is fine.

pasindud commented 11 years ago

@anatoliychakkaev is there anything more