LGTM On Thu, Jun 30, 2011 at 3:22 PM, Ryan Baxter <[email protected]> wrote:
> This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/974/ > > Ship it! > > LGTM > > > - Ryan > > On June 30th, 2011, 1:09 p.m., Stanton Sievers wrote: > Review request for shindig, johnfargo and Michael Hermanto. > By Stanton Sievers. > > *Updated 2011-06-30 13:09:14* > Description > > This change is to accommodate synchronous rpc handlers that may be registered > with the common container via > osapi.container.Container.prototype.rpcRegister. Per the documentation in > rpc.js#process, an rpc handler should return a result value that will be > included as part of a synchronous callback made by the rpc service. If no > result is provided it is assumed that the rpc handler will perform the > callback. > > Currently rpcRegister in the common container does not allow for the > returning of any results, thus forcing the rpc handler to perform the > callback even in synchronous cases. > > For convenience, here is the documentation in rpc.js#process: > // If there is a callback for this service, attach a callback function > // to the rpc context object for asynchronous rpc services. > // > // Synchronous rpc request handlers should simply ignore it and return a > // value as usual. > // Asynchronous rpc request handlers, on the other hand, should pass its > // result to this callback function and not return a value on exit. > // > // For example, the following rpc handler passes the first parameter back > // to its rpc client with a one-second delay. > // > // function asyncRpcHandler(param) { > // var me = this; > // setTimeout(function() { > // me.callback(param); > // }, 1000); > // } > > Testing > > Minimal testing performed. We noticed this problem during some feature > development in which a gadgets.rpc.call to the feature required a callback > and the rpc handler in the container did not explicitly invoke the callback. > This change fixed that problem. > > Diffs > > - > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > (1140140) > > View Diff <https://reviews.apache.org/r/974/diff/> >
