I took a quick peek and the code looks like the expected changes to switch jquery.couch.js to use jquery deferreds, which in my opinion, is a good addition. I haven't run the test suite with it, but assuming it doesn't break the existing callback based functionality, it would be an ubobtrusive and good feature to add.
For the changes to the urlPrefix functionality, I've got a couple questions about the statement: "removed the $.couch.urlPrefix, which wasn't working correctly anyway. since it wasn't working correctly i assume nobody is using it." What part of it isn't working? Currently urlPrefix allows you to specify a subdomain url for a database server as expected, and I've used it on many occasions. What the changes to urlPrefix in this pull request do appear to accomplish, is that by delaying the insertion of the base url from the initial creation of the db object [1] into the methods themselves ([2] for instance), you will then be able to set the url after you have created a db object. For instance, if you do: $.couch.urlPrefix = "http://me.mydb.com"; var db = $.couch.db("some_db") $.couch.urlPrefix = "http://otherdb.com"; In the current jquery.couch.js, urlPrefix will be hardcoded into the db variable, and further updates to $.couch.urlPrefix would not be brought in, whereas the patch would allow that to happen, which is arguablly a useful feature. My guess is that the "broken" functionality he was seeing was trying to set $.couch.urlPrefix after already having created a db object, which will not work. In my opinon, the name change from urlPrefix to url is an unneeded API change that will require code updates as people are currently using urlPrefix. The switch to deferreds should be unobtrusive, and keeping urlPrefix as the name will also keep the patches unobtrusive. Couple minor nits: The ajax deferred should be returned in [3]. Need to add $.couch.url in [4]. Overall looks like a good patch. I'm a big fan of jquery deferreds for ajax and they would be a welcome addition to jquery.couch.js, assuming that the test suite passes with this and the existing callbacks are unaffected. -Russell [1] https://github.com/apache/couchdb/pull/34/files#L0L284 [2] https://github.com/apache/couchdb/pull/34/files#L0R310 [3] https://github.com/apache/couchdb/pull/34/files#L0L316 [4] https://github.com/apache/couchdb/pull/34/files#L0L340 On Tue, Oct 23, 2012 at 6:24 AM, Noah Slater <[email protected]> wrote: > Can someone take a look at this? > > On 16 October 2012 18:23, boxxxie <[email protected]> wrote: > >> https://github.com/apache/couchdb/pull/34 > > > > > -- > NS
