incubator-couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filipe David Manana <fdman...@apache.org>
Subject Re: use view functions to emit changes
Date Mon, 06 Dec 2010 12:10:59 GMT
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?

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

As for 1), 2) and 3) I committed into my own branch:

https://github.com/fdmanana/couchdb/commit/1e5498abcdc005c24961685e37359809bf3fedad

good work
regards,


On Mon, Dec 6, 2010 at 8:36 AM, Benoit Chesneau <bchesneau@gmail.com> wrote:
> bump.
>
> On Tuesday, November 30, 2010, Benoit Chesneau <bchesneau@gmail.com> wrote:
>> On Tue, Nov 30, 2010 at 4:44 PM, Filipe David Manana
>> <fdmanana@apache.org> wrote:
>>>>
>>>> Here is an updated patch. It now use a filter sandbox instead of
>>>> patching the function and it fixes whitespaces.
>>>
>>> Why a separate function to init the filter_sandbox? It can be done at
>>> the end of the existing init_sandbox() function (perhaps renaming it
>>> to init_sandboxes).
>>>
>>> I would rename Filter.filter_view to Filter.view_filter or to
>>> Filter.map_filter (doing the same in
>>> src/couchdb/couch_query_servers.erl for naming consistency).
>>>
>>> 4) is still there (the debugging ?LOG_INFO line).
>>>
>>> Also, this patch should only contain the changes necessary to
>>> implement the new feature. While all the whitespace and indentation
>>> fixes (to comply with the wiki rules) are a welcome plus, they should
>>> come in a separate patch imo.
>>>
>> Well I used filter_view for consistency, since we use filter_docs
>> already, I don't think it need to be changed at this point. I've
>> removed the log stuff.
>>
>> Are we OK for such feature ? If yes, I will commit it.
>>
>> - benoit
>>
>



-- 
Filipe David Manana,
fdmanana@gmail.com, fdmanana@apache.org

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Mime
View raw message