Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(928)

Issue 4295051: Copy gadget and container in CacheEnforcementVisitor (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years ago by nikhilmadan23
Modified:
14 years, 11 months ago
Reviewers:
gagan.goku, dev-remailer
CC:
cool-shindig-committers_googlegroups.com, atulvasu
Base URL:
http://svn.apache.org/repos/asf/shindig/trunk/
Visibility:
Public.

Description

Copy gadget and container of the original request in CacheEnforcementVisitor while triggering fetches.

Patch Set 1 #

Patch Set 2 : Readability changes #

Patch Set 3 : More readability changes #

Patch Set 4 : Remove import #

Total comments: 4

Patch Set 5 : Addressing comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -15 lines) Patch
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java View 1 2 3 4 5 chunks +35 lines, -15 lines 0 comments Download
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitorTest.java View 1 2 3 4 3 chunks +28 lines, -0 lines 0 comments Download

Messages

Total messages: 9
nikhilmadan23
15 years ago (2011-03-15 10:20:56 UTC) #1
atulvasu
LGTM
15 years ago (2011-03-15 10:31:05 UTC) #2
gagan.goku
Looks good. http://codereview.appspot.com/4295051/diff/11001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java (right): http://codereview.appspot.com/4295051/diff/11001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java#newcode89 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java:89: request.setGadget(spec.getUrl()); this conflicts with http://codereview.appspot.com/4277050/diff/1/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java which 1 ...
15 years ago (2011-03-15 10:40:37 UTC) #3
gagan.goku
http://codereview.appspot.com/4295051/diff/11001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java (right): http://codereview.appspot.com/4295051/diff/11001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java#newcode84 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java:84: protected static HttpRequest createNewHttpRequest(Gadget gadget, String uriStr) { make ...
15 years ago (2011-03-15 10:43:06 UTC) #4
nikhilmadan23
http://codereview.appspot.com/4295051/diff/11001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java (right): http://codereview.appspot.com/4295051/diff/11001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java#newcode89 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java:89: request.setGadget(spec.getUrl()); On 2011/03/15 10:40:37, gagan.goku wrote: > this conflicts ...
15 years ago (2011-03-15 10:43:17 UTC) #5
atulvasu
I'll revert my CL he does all that I needed :) On Tue, Mar 15, ...
15 years ago (2011-03-15 10:43:27 UTC) #6
nikhilmadan23
Addressing comments
15 years ago (2011-03-15 10:52:04 UTC) #7
nikhilmadan23
http://codereview.appspot.com/4295051/diff/11001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java (right): http://codereview.appspot.com/4295051/diff/11001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java#newcode84 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CacheEnforcementVisitor.java:84: protected static HttpRequest createNewHttpRequest(Gadget gadget, String uriStr) { On ...
15 years ago (2011-03-15 10:52:22 UTC) #8
gagan.goku
15 years ago (2011-03-15 12:57:38 UTC) #9
Build looks good.
Committed as r1081754.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b