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

Issue 4003047: CacheEnforcementVisitor is added to script & css concat rewriters (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 1 month ago by pulkitgoyal2000
Modified:
15 years, 1 month ago
Reviewers:
johnfargo, dev, dev-remailer, plindner1, gagan.goku
Base URL:
http://svn.apache.org/repos/asf/shindig/trunk/
Visibility:
Public.

Description

CacheEnforcementRewriter is added to relevant rewriters so that only non-private & cached resources are proxied or modified.

Patch Set 1 #

Total comments: 7

Patch Set 2 : Addressing Comment #

Total comments: 4

Patch Set 3 : Addressing Comments #

Total comments: 4

Patch Set 4 : Addressing Comments #

Total comments: 2

Patch Set 5 : Adding Comments #

Patch Set 6 : Remove unused variable #

Patch Set 7 : Mischunk error #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -13 lines) Patch
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java View 1 2 3 4 5 6 1 chunk +23 lines, -6 lines 0 comments Download
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleConcatContentRewriter.java View 1 2 3 4 5 6 1 chunk +23 lines, -7 lines 0 comments Download

Messages

Total messages: 12
pulkitgoyal2000
15 years, 1 month ago (2011-01-24 10:41:46 UTC) #1
nikhilmadan23
http://codereview.appspot.com/4003047/diff/1/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java (right): http://codereview.appspot.com/4003047/diff/1/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java#newcode47 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java:47: import java.util.concurrent.Executor; Order alphabetically. http://codereview.appspot.com/4003047/diff/1/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java#newcode51 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ImageAttributeRewriter.java:51: import java.util.Arrays; Order ...
15 years, 1 month ago (2011-01-24 11:30:19 UTC) #2
pulkitgoyal2000
Removed ImageAttibuteRewriter.java as it is not a stable rewriter. http://codereview.appspot.com/4003047/diff/1/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java (right): http://codereview.appspot.com/4003047/diff/1/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java#newcode26 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java:26: ...
15 years, 1 month ago (2011-01-25 06:08:19 UTC) #3
nikhilmadan23
lgtm Could you add a comment near the class definition saying that you are using ...
15 years, 1 month ago (2011-01-25 09:26:02 UTC) #4
pulkitgoyal2000
http://codereview.appspot.com/4003047/diff/9001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java (right): http://codereview.appspot.com/4003047/diff/9001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java#newcode50 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java:50: On 2011/01/25 09:26:02, nikhilmadan23 wrote: > Extra blank line. ...
15 years, 1 month ago (2011-01-25 09:42:38 UTC) #5
nikhilmadan23
lgtm Could you add Anupama to the reviewers list.
15 years, 1 month ago (2011-01-25 09:53:32 UTC) #6
anupama.dutta
http://codereview.appspot.com/4003047/diff/17001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java (right): http://codereview.appspot.com/4003047/diff/17001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java#newcode56 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java:56: @Named("shindig.concat.executor") Executor executor) { Shall we order these params ...
15 years, 1 month ago (2011-01-25 10:28:26 UTC) #7
satya3656
Can you change the issue title to "CacheEnforcementVisitor is added to script & css concat ...
15 years, 1 month ago (2011-01-26 07:47:22 UTC) #8
pulkitgoyal2000
http://codereview.appspot.com/4003047/diff/17001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java (right): http://codereview.appspot.com/4003047/diff/17001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java#newcode56 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java:56: @Named("shindig.concat.executor") Executor executor) { On 2011/01/25 10:28:26, anupama.dutta wrote: ...
15 years, 1 month ago (2011-01-27 08:14:48 UTC) #9
anupama.dutta
LGTM. Please address the small nit mentioned below and send it to dev@ with Fargo, ...
15 years, 1 month ago (2011-01-27 15:36:58 UTC) #10
pulkitgoyal2000
http://codereview.appspot.com/4003047/diff/23001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java (right): http://codereview.appspot.com/4003047/diff/23001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java#newcode37 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ScriptConcatContentRewriter.java:37: * REwrites scripts. On 2011/01/27 15:36:59, anupama.dutta wrote: > ...
15 years, 1 month ago (2011-01-31 05:33:18 UTC) #11
gagan.goku
15 years, 1 month ago (2011-02-01 10:19:29 UTC) #12
Build looks good.
Committed as r1065983.
Sign in to reply to this message.

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