ping?

On 2012/01/03 16:27:51, conroy wrote:

http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/HostChannel.cpp
File plugins/common/HostChannel.cpp (right):


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/HostChannel.cpp#newcode322
plugins/common/HostChannel.cpp:322: bool
HostChannel::readValue(gwt::Value&
valueRef) {
alternatively, for less code churn you could just add a using
gwt::Value at the
top (here and elsewhere)


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/InvokeSpecialMessage.h
File plugins/common/InvokeSpecialMessage.h (right):


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/InvokeSpecialMessage.h#newcode44
plugins/common/InvokeSpecialMessage.h:44:
InvokeSpecialMessage(SessionHandler::SpecialMethodId dispatchId, int
numArgs,
const gwt::Value* args) : dispatchId(dispatchId),
nit: over 100 chars.


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/SessionHandler.h
File plugins/common/SessionHandler.h (right):


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/SessionHandler.h#newcode87
plugins/common/SessionHandler.h:87: virtual bool invoke(HostChannel&
channel,
const gwt::Value& thisObj, const std::string& methodName,
nit: 100chars


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/Value.h
File plugins/common/Value.h (right):


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/Value.h#newcode30
plugins/common/Value.h:30: namespace gwt {
it seems strange to put only Value in this namespace--shouldn't the
rest of our
local implementation classes also be part of this namespace?


alternatively, consider just renaming Value...e.g. GwtValue


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/Value.h#newcode397
plugins/common/Value.h:397: }
// namespace gwt


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/xpcom/Makefile
File plugins/xpcom/Makefile (right):


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/xpcom/Makefile#newcode153
plugins/xpcom/Makefile:153: $(error Unrecognized BROWSER of $(BROWSER)
- options
are ff3, ff3+, ff35, ff36, ff40, ff50, ff60, ff70, f80, f90)
ff80, ff90


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/xpcom/Preferences.cpp
File plugins/xpcom/Preferences.cpp (right):


http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/xpcom/Preferences.cpp#newcode30
plugins/xpcom/Preferences.cpp:30: using namespace gwt;
only pull in specific types with a using decl



http://gwt-code-reviews.appspot.com/1620803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to