LGTM with nits.

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58001
File plugins/npapi/DevModeOptions/.classpath (right):

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58001#newcode3
plugins/npapi/DevModeOptions/.classpath:3: <classpathentry kind="src"
path="src"/>
Shouldn't this use a linked resource relate to GWT_ROOT, and the eclipse
files be under $GWT_ROOT/eclipse?

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58012
File
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/client/LocalStorage.java
(right):

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58012#newcode56
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/client/LocalStorage.java:56:
return this.setItem(key, JSON.stringify(dataObject));
Why return for a void method?

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58022
File plugins/npapi/VisualStudio/npapi-plugin.sln (right):

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58022#newcode3
plugins/npapi/VisualStudio/npapi-plugin.sln:3: # Visual Studio 2008
Are we making a conscious decision to drop support for VS2005?

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58023
File plugins/npapi/VisualStudio/npapi-plugin.vcproj (right):

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58023#newcode74
plugins/npapi/VisualStudio/npapi-plugin.vcproj:74:
DataExecutionPrevention="0"
Why not DEP if we are switching to VS2008?

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58024
File plugins/npapi/main.cpp (right):

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58024#newcode26
plugins/npapi/main.cpp:26: DisableThreadLibraryCalls(hModule);
Spacing.

http://gwt-code-reviews.appspot.com/1084801/diff/57001/58024#newcode242
plugins/npapi/main.cpp:242: return 0 ;
IIUC, this means we silently ignore any events rather than report them
as unhandled -- is that right?

http://gwt-code-reviews.appspot.com/1084801/show

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

Reply via email to