incubator-couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benoit Chesneau <bchesn...@gmail.com>
Subject Re: use view functions to emit changes
Date Tue, 14 Dec 2010 12:42:43 GMT
Back on this, have been distracted by flu.

On Mon, Dec 6, 2010 at 1:10 PM, Filipe David Manana <fdmanana@apache.org> wrote:
> Sorry for the delay BenoƮt.
>
> Comments:
>
> 1) At https://github.com/benoitc/couchdb/compare/master...view_changes#L1R40
> you're also changing the emit of the regular sandbpox (don't forget,
> in the previous statement you're copying a reference, not a value):
> E.g.:
>
> $ ./couchjs -
> c1 = evalcx('');
> c1.foo = "bar";
> c2 = c1;
> c2.foo = "hello world";
> print("c1.foo: " + c1.foo);
> print("c2.foo: " + c2.foo);
> <EOF>
> c1.foo: hello world
> c2.foo: hello world
>
> 2) couch_changes:filter_view/4 has an unused parameter in both clauses
> (Req) - it's better to remove it?
>

Yes
> 3) The error message at
> https://github.com/benoitc/couchdb/compare/master...view_changes#L1R40
> uses single quotes. Everywhere else we use back ticks.
>
> 4) Perhaps here:
> https://github.com/benoitc/couchdb/compare/master...view_changes#L4R235
> the name of the command sent to the view server should be something
> like 'view_filter' instead of 'views', which to me is a bit misleading

Well it would require more changes in loop.js. It actually looks for
mountpoints in the design doc to validate it. Her views is the mmember
of the ddoc where to find the map function. I think we should keep  it
/

>
> As for 1), 2) and 3) I committed into my own branch:
>
> https://github.com/fdmanana/couchdb/commit/1e5498abcdc005c24961685e37359809bf3fedad


Applied and tested on mine:

https://github.com/benoitc/couchdb/tree/view_changes


tests pass.
>
> good work
> regards,
>
>
Thanks for the review again :) .


OK for this patch ?

- benoit

Mime
View raw message