You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by boxxxie <gi...@git.apache.org> on 2012/10/16 19:23:59 UTC

couchdb pull request: Many fixes to $.couch

GitHub user boxxxie opened a pull request:

    https://github.com/apache/couchdb/pull/34

    Many fixes to $.couch

    added url param (allows to use $.couch to hit subdomains)
    all $.couch functions return jQuery promises (basically added returns on all the functions, cept for the userDb function which was edited the most).
    fixed error in uuid function where calling it multiple times had it return different types of values (sometimes a string, sometimes an object).
    
    removed the $.couch.urlPrefix, which wasn't working correctly anyway. since it wasn't working correctly i assume nobody is using it.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/boxxxie/couchdb master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb/pull/34.patch

----
commit d17cc498e2abcd69e10ae8bbad7f521e6c32b86a
Author: Paul Iannazzo <so...@gmail.com>
Date:   2012-10-04T11:12:52-07:00

    patched jquery.couch to return it's ajax calls so as to use promises provided by $.ajax

commit bd80f791553bbe908d0474dea8b92967d1623617
Author: Paul Iannazzo <so...@gmail.com>
Date:   2012-10-12T14:41:39-07:00

    removed the prefix adding on session ajax call

commit 6a9b7deffc3665604828ddb1591a4df5c0e6eb86
Author: Paul Iannazzo <so...@gmail.com>
Date:   2012-10-12T14:48:38-07:00

    removed all urlPrefix additions to ajax calls
    they were causing duplicate prefixes to be added because jquery is probably doing it on it's own and the couch jquery code is redundant

commit fbde4734078299d14c77625118b394dbf0bf1f50
Author: Paul Iannazzo <so...@gmail.com>
Date:   2012-10-15T18:47:12-07:00

    edited $.couch session functions to use a url from the options if it exists

commit b17e931aab2dfbee46f4a07d90b0e436d6da4d53
Author: Paul Iannazzo <so...@gmail.com>
Date:   2012-10-16T09:53:30-07:00

    added $.couch.url to all ajax calls as a prefix. it has default settings that should not break things :)

commit 4289edcbfda93d06d6e77050582a33ab7e84ebc9
Author: Paul Iannazzo <so...@gmail.com>
Date:   2012-10-16T10:19:15-07:00

    Merge git://git.apache.org/couchdb

----


Re: couchdb pull request: Many fixes to $.couch

Posted by "john.tiger" <jo...@gmail.com>.
thks for info - not entirely sure how this will integrate with the 
upcoming CORS commit but when that appears, we will try to use all of it 
in a non-couchapp html5 program - previously we could not get things 
running with lots of cross-domain errors, etc.  Should make for a good 
test and a nice how-to/tutorial to go with CORS feature whether it makes 
1.3 or 1.4.

On 10/23/2012 11:37 AM, Russell Branca wrote:
> 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 <ns...@apache.org> wrote:
>> Can someone take a look at this?
>>
>> On 16 October 2012 18:23, boxxxie <gi...@git.apache.org> wrote:
>>
>>> https://github.com/apache/couchdb/pull/34
>>
>>
>>
>> --
>> NS


Re: couchdb pull request: Many fixes to $.couch

Posted by Russell Branca <ch...@gmail.com>.
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 <ns...@apache.org> wrote:
> Can someone take a look at this?
>
> On 16 October 2012 18:23, boxxxie <gi...@git.apache.org> wrote:
>
>> https://github.com/apache/couchdb/pull/34
>
>
>
>
> --
> NS

Re: couchdb pull request: Many fixes to $.couch

Posted by Noah Slater <ns...@apache.org>.
Can someone take a look at this?

On 16 October 2012 18:23, boxxxie <gi...@git.apache.org> wrote:

> https://github.com/apache/couchdb/pull/34




-- 
NS