> On 2012-04-26 19:58:44, Henry Saputra wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js, > > line 436 > > <https://reviews.apache.org/r/4874/diff/5/?file=104573#file104573line436> > > > > Why change from === to == ?
It only looks like I changed it, the original one isn't aligning correctly in this diff, it's further down. === is the exact same object and == is just as good, if not better for the spirit of what we're doing here. I really think we're way overusing === in shindig, but I left the original check as-is. > On 2012-04-26 19:58:44, Henry Saputra wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js, > > line 179 > > <https://reviews.apache.org/r/4874/diff/5/?file=104574#file104574line179> > > > > I thought this is a test to disable refresh? Why would we change it to > > GET? Cause the shindig server will set no-cache headers (based on the request to not cache it) and tell our browser not to cache it. There's no reason to redirect it through a post just because we want no refresh. - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4874/#review7272 ----------------------------------------------------------- On 2012-04-26 18:33:29, Dan Dumont wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4874/ > ----------------------------------------------------------- > > (Updated 2012-04-26 18:33:29) > > > Review request for shindig, Ryan Baxter, Stanton Sievers, and Brian Lillie. > > > Summary > ------- > > So I think this will clean up makeRequest from the client's perspective. > > The implications of this are that if we use the refreshInterval param we > should not return the original response but instead return the response that > has the clobbered cache headers. This is because we are processing the get > as-is and returned cache-control headers need to be valid for what we've > requested. > > There is currently an issue in > org.apache.shindig.gadgets.http.AbstractHttpCache where when building the > response to cache, we clobber the cache-control headers and do not return the > modified response. If these changes listed here for makeRequest make sense, > we can continue and fix this other problem. > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpCache.java > 1329959 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java > 1329959 > > Diff: https://reviews.apache.org/r/4874/diff > > > Testing > ------- > > Updated tests. All tests pass. > > > Thanks, > > Dan > >
