Back on this, have been distracted by flu. On Mon, Dec 6, 2010 at 1:10 PM, Filipe David Manana <fdman...@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
