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, 30 Nov 2010 12:05:47 GMT
And thanks for the review anyway!

- benoît

On Tue, Nov 30, 2010 at 1:05 PM, Benoit Chesneau <bchesneau@gmail.com> wrote:
> On Tue, Nov 30, 2010 at 12:22 PM, Filipe David Manana
> <fdmanana@apache.org> wrote:
>> Hi Benoît,
>>
>> It's a good idea I think. Good work.
>> I have some comments regarding the patch:
>>
>> 1) Doing a regexp substitution of the view function's code string
>> seems like a recipe for disaster to me.
>>
>> I think the safe and clean way to go here is to create another
>> sandbox, with a different 'emit' function, and in
>> Couch.compileFunction (utils.js) pass a third and optional argument
>> which is the context in which the function is going to be executed. If
>> omitted, it uses 'sandbox' as the context, otherwise use that other
>> context. Then here:
>>
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L0R34
>>
>> You would so something like:
>>
>> fun = Couch.compileFunction(source, filter_sandbox);
>>
>
> Indeed that's better, will make the change.
>>
>> 2) Don't forget the indentation level for .js files is 2 spaces (I'm
>> seeing a mix of 4 and 2 spaces in share/server/filter.js)
>>
>> 3) Avoid the unnecessary white-space only changes:
>>
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L2R396
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L2R477
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L3R255
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L4R60
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L4R85
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L4R247
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L4R503
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L3R191
>
>
> about 2 & 3 can we add indent rules on top of our sources from now ?
> I think there is something common between vim and emacs for example.
> That should solve such errors. I don't want to change my config each
> time I'm using a project. Using such rules solves that automatically.
>
> Ex:
>
> %% -*- tab-width: 4;erlang-indent-level: 4;indent-tabs-mode: nil -*-
> %% ex: ts=4 sw=4 et
>
>
> works on erlang and vim and surely other editors. About the 2 spaces
> in js, imo we should go to a 4 spaces indentations, which is a way
> more readable and more common. (Mozilla uses that rule.)
>
>>
>> 4) This?LOG_INFO line is there for your debugging purposes I think
>> (should be removed):
>>
>> https://github.com/benoitc/couchdb/commit/651e29e1bb767fb493bf75497623dae89bf9a5ad#L3R168
>>
>>
> yup.
>

Mime
View raw message