LGTM Seems reasonable. Patching.
On Wed, Apr 7, 2010 at 11:11 AM, Ziv Horesh <[email protected]> wrote: > On Wed, Apr 7, 2010 at 11:03 AM, <[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 > > >
