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:08 GMT
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