> On 2011-12-19 18:23:54, Dan Dumont wrote: > > So far I have 1 vote for splitting the update function into 2 parts (from > > Jesse), and 1 for leaving it as 1 function (from Henry, I think on the dev > > list). > > > > Are there any other opinions on the matter? > > Ryan Baxter wrote: > I agree with Jesse on this one. Splitting this up may help make this > more understandable. I see your point about not having to set the token but > this can be made clear in the specification for the common container. It > took me a little bit to figure out what was happening, and I am still not > sure I completely understand it all... > > Ryan Baxter wrote: > Just so it is clear, I don't think it will be the end of the world if we > don't have an separate set function. I would really like to see more helper > functions. We all have to read through the code to understand things in > Shindig and reading this code is kind of difficult, so anything we can do to > make it more strait forward would be good.
Agreed -- I think having two functions would make the implementation simpler, which was actually another motivator for my comment about breaking the function up in the first place (although I didn't call that out explicitly, which I guess I probably should have). It took me a while to figure out what was going on in the current implementation too. - Jesse ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3180/#review3976 ----------------------------------------------------------- On 2011-12-14 16:35:00, Dan Dumont wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3180/ > ----------------------------------------------------------- > > (Updated 2011-12-14 16:35:00) > > > Review request for shindig, Henry Saputra, Ryan Baxter, li xu, Jesse > Ciancetta, and Stanton Sievers. > > > Summary > ------- > > Initial review of 1st change. Allowing common container to manage container > token refreshes. Also, refresh of gadget security tokens will now wait for > valid container security token before trying to refresh. > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1213887 > > Diff: https://reviews.apache.org/r/3180/diff > > > Testing > ------- > > Tested code in a private container with some examples of setting no refresh > (ttl = 0) and setting an initial token (if it was written by jsp page to > avoid transaction) etc.. > > > Thanks, > > Dan > >
