Thanks for taking a look. I've updated my client.
I think the dev/ comments have been subsumed by the latest version of the patch at: http://gwt-code-reviews.appspot.com/46801/show > bad indent in DynaTable.gwt.xml Fixed. > CustomFieldSerializerValidator looks like checkstyle fixing; separate > submit? Ok. > ProxyCreator:337, prexisting modulo your refactoring, but the logger message > of "" seems sub-optimal to me. Fixed. > the com.google.gwt.rpc.*Test2 family of tests seem poorly named. They may > be adequately differentiated from the older suites by the package name, but > I'm worried that e.g. searches for *Test.java will miss them unexpectedly... > if you want a different name, can the suffix go before "Test"? E.g. maybe > "DeRpcTest"? Renamed as Rpc*Test. > IdentityValueCommand: the point to the methods here is just to finalize > them? Yes. This has been done to make sure that any other types added in the future (like inline protocol buffers) have the correct semantics. javadoc / impls updated. > CommandClientSerializationStreamWriter:73: am I reading that only arrays, > default objects, and custom field serialized objects are put into > identityMap? Should the check for presence in that map be deferred, if e.g. > primitives aren't stored in it? Reordered. > Pair: shouldn't there be a common place for this, outside the deRPC server > context? There should be, but I don't really know where it would go. I would have put this in the dev.util.collect package, but that's not bundled with gwt-user or gwt-servlet. > RPCServlet.java: Since we need matching e.g. X-GWT-Module-Base strings in > multiple places, should we have a shared Constants class for them? Right now gwt-servlet.jar has all of the client code packed into it. If this were to change, the Constants class would cause runtime-only breaks. > RPCServlet.java:182: I realize processCall() is small replicated code, but > shouldn't we just have out be an OutputStream, and consult DUMP_PAYLOAD > twice (before and after) so as to not replicate? Done. > RPCServlet.java:270: javadoc doesn't match implementation; gzips for !local, > regardless of size. Fixed. -- Bob Vawter Google Web Toolkit Team --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
