Set encoding type for HttpResponseBuilder when setting from string

Issue found due to rewritters change.
Basically when a new response is created based on current response with encoding different then utf8, and then setResponseString is called, the encoding would be wrong.

The solution is to update encoding when calling setResponseString.

[johnfargo, 2010-04-07 18:03:14]

one very quick q.

http://codereview.appspot.com/816045/diff/1/3
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java
(right):

http://codereview.appspot.com/816045/diff/1/3#newcode79
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java:79:
public HttpResponseBuilder setEncoding(Charset charset) {
looks like we intend to make this an implementation detail. package-private for
testing purposes?

[zhoresh, 2010-04-07 18:12:15]

On Wed, Apr 7, 2010 at 11:03 AM, <johnfargo@gmail.com> wrote:

> one very quick q.
>
>
> http://codereview.appspot.com/816045/diff/1/3
> File
>
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java
> (right):
>
> http://codereview.appspot.com/816045/diff/1/3#newcode79
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java:79:
> public HttpResponseBuilder setEncoding(Charset charset) {
> looks like we intend to make this an implementation detail.
> package-private for testing purposes?

Since this is a builder, I can see cases that someone might want to use it,
so I don't see a reason why to hide it.
I don't see it as implementation detail, but more of a service.


>
>
> http://codereview.appspot.com/816045/show
>

[fargo, 2010-04-07 18:31:04]

LGTM

Seems reasonable. Patching.

On Wed, Apr 7, 2010 at 11:11 AM, Ziv Horesh <zhoresh@gmail.com> wrote:

> On Wed, Apr 7, 2010 at 11:03 AM, <johnfargo@gmail.com> wrote:
>
> > one very quick q.
> >
> >
> > http://codereview.appspot.com/816045/diff/1/3
> > File
> >
> >
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java
> > (right):
> >
> > http://codereview.appspot.com/816045/diff/1/3#newcode79
> >
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java:79:
> > public HttpResponseBuilder setEncoding(Charset charset) {
> > looks like we intend to make this an implementation detail.
> > package-private for testing purposes?
>
> Since this is a builder, I can see cases that someone might want to use it,
> so I don't see a reason why to hide it.
> I don't see it as implementation detail, but more of a service.
>
>
> >
> >
> > http://codereview.appspot.com/816045/show
> >
>

[johnfargo, 2010-04-07 18:34:57]

Committed @931639.

On 2010/04/07 18:31:04, fargo wrote:
> LGTM
> 
> Seems reasonable. Patching.
> 
> On Wed, Apr 7, 2010 at 11:11 AM, Ziv Horesh <mailto:zhoresh@gmail.com> wrote:
> 
> > On Wed, Apr 7, 2010 at 11:03 AM, <mailto:johnfargo@gmail.com> wrote:
> >
> > > one very quick q.
> > >
> > >
> > > http://codereview.appspot.com/816045/diff/1/3
> > > File
> > >
> > >
> > >
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java
> > > (right):
> > >
> > > http://codereview.appspot.com/816045/diff/1/3#newcode79
> > >
> > >
> >
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseBuilder.java:79:
> > > public HttpResponseBuilder setEncoding(Charset charset) {
> > > looks like we intend to make this an implementation detail.
> > > package-private for testing purposes?
> >
> > Since this is a builder, I can see cases that someone might want to use it,
> > so I don't see a reason why to hide it.
> > I don't see it as implementation detail, but more of a service.
> >
> >
> > >
> > >
> > > http://codereview.appspot.com/816045/show
> > >
> >
>