- generally, there's several line-wrap changes; did you mean them? Some
seem to be desirably shortening very long lines, but a few length lines
(E.g. javadoc in JDeclaredType l104:106). Minor, though.
- unqualified TODO at GenerateJavaAST:2980, what are you
promising/offering to do here? But most of those changes seem to be
subsumed in Scott's review comments, so I'll skip to that thread for others.
Most of which he already covered.
- Generically, I at least could use a lot more javadoc (or at least
doc) in the compiler parts (dev/*/**.java). I'd like to see new code
improve that, at least.
- If we don't want to support ArtificialRescue, should we move it into
an impl package?
- bad indent in DynaTable.gwt.xml
- CustomFieldSerializerValidator looks like checkstyle fixing; separate
submit?
- ProxyCreator:337, prexisting modulo your refactoring, but the logger
message of "" seems sub-optimal to me.
- 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"?
- IdentityValueCommand: the point to the methods here is just to finalize
them?
- 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?
- Pair: shouldn't there be a common place for this, outside the deRPC
server context?
- 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?
- 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?
- RPCServlet.java:270: javadoc doesn't match implementation; gzips for
!local, regardless of size.
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
