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:[email protected]>
wrote:
> On Wed, Apr 7, 2010 at 11:03 AM, <mailto:[email protected]> 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
> >
>
http://codereview.appspot.com/816045/show