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

Issue 1232044: Use UriCommon from AbstractHttpCache

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 10 months ago by zhoresh
Modified:
15 years, 10 months ago
Reviewers:
henry.saputra, shindig.remailer
Base URL:
http://svn.apache.org/repos/asf/shindig/trunk
Visibility:
Public.

Description

Simple dependency cleanup and use of UriCommon instead of ImageRewriter.

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -7 lines) Patch
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java View 2 chunks +4 lines, -7 lines 1 comment Download

Messages

Total messages: 3
zhoresh
15 years, 10 months ago (2010-05-20 15:58:58 UTC) #1
henry.saputra
LGTM with small comment. http://codereview.appspot.com/1232044/diff/1/2 File java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java (right): http://codereview.appspot.com/1232044/diff/1/2#newcode152 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java:152: .setParam("rh", request.getParam(UriCommon.Param.RESIZE_HEIGHT.getKey())) Since the UriCommon.Param ...
15 years, 10 months ago (2010-05-20 16:20:28 UTC) #2
zhoresh
15 years, 10 months ago (2010-05-20 17:29:26 UTC) #3
Thanks. Updated and committed.

On Thu, May 20, 2010 at 9:20 AM, <henry.saputra@gmail.com> wrote:

> LGTM with small comment.
>
>
> http://codereview.appspot.com/1232044/diff/1/2
> File
>
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
> (right):
>
> http://codereview.appspot.com/1232044/diff/1/2#newcode152
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java:152:
>
> .setParam("rh",
> request.getParam(UriCommon.Param.RESIZE_HEIGHT.getKey()))
> Since the UriCommon.Param enums are used simply as String to get data
> from request parameter, maybe could use private final String to store
> the params like in BasicImageRewriter class.
>
>
> http://codereview.appspot.com/1232044/show
>
Sign in to reply to this message.

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