Sorry for the delay. I was on vacation the day after I sent this out.
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) { On 2012/01/03 16:27:51, conroy wrote:
alternatively, for less code churn you could just add a using
gwt::Value at the
top (here and elsewhere)
Oh. I didn't know you can use "using" for just a single name from a namespace. But after spending all my time debugging weird name collision errors, I think I'd stick to referring to it using the full qualify name 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), On 2012/01/03 16:27:51, conroy wrote:
nit: over 100 chars.
Done. 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, On 2012/01/03 16:27:51, conroy wrote:
nit: 100chars
Done. 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 { On 2012/01/03 16:27:51, conroy wrote:
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
I contemplated on both of those before and almost went with GwtValue. I decided to go with gwt:Value since it can be less verbose with "using namespace gwt" http://gwt-code-reviews.appspot.com/1620803/diff/1/plugins/common/Value.h#newcode397 plugins/common/Value.h:397: } On 2012/01/03 16:27:51, conroy wrote:
// namespace gwt
Done. 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) On 2012/01/03 16:27:51, conroy wrote:
ff80, ff90
Done. Good catch. 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; On 2012/01/03 16:27:51, conroy wrote:
only pull in specific types with a using decl
Actually this is not needed. http://gwt-code-reviews.appspot.com/1620803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
