I fixed all the stuff that wasn't related to default handling of json
params by protocol handlers, because several of those suggestions were
broke, and didn't have a chance yet to figure out why they are broken on
the server.

Still, wanted to get the first pass checked in, and I can figure out the
problems when I get back.



http://codereview.appspot.com/14109/diff/1/14
File features/features.txt (right):

http://codereview.appspot.com/14109/diff/1/14#newcode23
Line 23: features/opensocial.data/feature.xml
On 2009/02/20 01:19:12, louiscryan wrote:
opensocial.data and opensocial-data dont mix well. Time for spec
cleanup fun!

Done.

http://codereview.appspot.com/14109/diff/1/28
File features/opensocial.data.activities/activitiestest.js (right):

http://codereview.appspot.com/14109/diff/1/28#newcode27
Line 27: shindig.auth = shindig.auth || {};
On 2009/02/24 21:06:43, awiner wrote:
i think shindig.auth should just be overwritten here with {}.  In
part, that
verifies that nothing out of shindig.auth other than getSecurityToken
is called.

There are some other weird things going on, but overwrote it in all
tests, and then undefined it, because any test using shindig.auth should
ensure it's existence.
Done.

http://codereview.appspot.com/14109/diff/1/28#newcode28
Line 28: shindig.auth.getSecurityToken = shindig.auth.getSecurityToken
|| function() {
On 2009/02/24 21:06:43, awiner wrote:
think it should just be overridden unconditionally

Done.

http://codereview.appspot.com/14109/diff/1/28#newcode38
Line 38: shindig.auth = undefined;
On 2009/02/24 21:06:43, awiner wrote:
i think the original shindig.auth should be restored

Done.

http://codereview.appspot.com/14109/diff/1/28#newcode42
Line 42: *
On 2009/02/24 21:06:43, awiner wrote:
cool comment ;)

Done.

http://codereview.appspot.com/14109/diff/1/28#newcode138
Line 138: [{title:"yellow",userId:"john.doe",id:"1",body:"what a
color!"}],
On 2009/02/24 21:06:43, awiner wrote:
JSON formatting is widely divergent within this file.  pick a style?

Done.

http://codereview.appspot.com/14109/diff/1/35
File features/opensocial.data.base/jsonrequest.js (right):

http://codereview.appspot.com/14109/diff/1/35#newcode56
Line 56: "CONTENT_TYPE" : "JSON",
On 2009/02/20 01:19:12, louiscryan wrote:
JSON_CONTENT_TYPE instead of "JSON ?

Done.

http://codereview.appspot.com/14109/diff/1/35#newcode124
Line 124: var normalizeSortAndFilterOptionNames = function(params) {
On 2009/02/20 01:19:12, louiscryan wrote:
no need to do this any more

Done.

http://codereview.appspot.com/14109/diff/1/22
File features/test/internal/testutils.js (right):

http://codereview.appspot.com/14109/diff/1/22#newcode51
Line 51: var makeInspectableCallback = function() {
On 2009/02/24 21:06:43, awiner wrote:
pass callback to this function, instead of setRealCallback() - it's
always used,
right?

Done.

http://codereview.appspot.com/14109/diff/1/22#newcode62
Line 62: realCallback(result);
On 2009/02/24 21:06:43, awiner wrote:
i'd use
   realCallback.apply(this, args)
... so this will work for any set of arguments, with "this" preserved.

Done.

http://codereview.appspot.com/14109

Reply via email to