> On 2012-05-10 12:38:56, Ryan Baxter wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js, > > line 449 > > <https://reviews.apache.org/r/5085/diff/2/?file=108271#file108271line449> > > > > I prefer we use the new constant here > > Erik Bi wrote: > the reason I still see the parameter name instead the constant is because > I want to to keep constant with other code in the makeRequest function, as > you can see, in this function, it checks many parameters, but all use the > string value directly instead of the constant. your opinion? > > Ryan Baxter wrote: > Just because the rest of the code does it doesn't mean its the best thing > to do. At least by using the constants you are "testing" them and making > sure their values are what you expect.
Make sense. I will change it to using constant. - Erik ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5085/#review7765 ----------------------------------------------------------- On 2012-05-14 12:39:40, Erik Bi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5085/ > ----------------------------------------------------------- > > (Updated 2012-05-14 12:39:40) > > > Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. > > > Summary > ------- > > The OpenSocial spec says that you should be able to pass 'SIGN_OWNER' and > 'SIGN_VIEWER' as parameters to makeRequest. Shindig does not define these and > in fact looks for the wrong parameters. In io.js Shindig looks for > 'OWNER_SIGNED' and 'VIEWER_SIGNED'. In addition 'SIGN_OWNER' and > 'SIGN_VIEWER' should be made constants. > > > This addresses bug shindig-1772. > https://issues.apache.org/jira/browse/shindig-1772 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js > 1327432 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/proxied-form-post/post.js > 1327432 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js > 1327432 > > Diff: https://reviews.apache.org/r/5085/diff > > > Testing > ------- > > Update iotest.js > > > Thanks, > > Erik > >
