http://gwt-code-reviews.appspot.com/1084801/diff/6001/7006
File
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/DevModeOptions.gwt.xml
(right):

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7006#newcode5
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/DevModeOptions.gwt.xml:5:
<inherits name='com.google.gwt.dom.DOM' />
User should automatically include DOM & Core.

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7006#newcode12
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/DevModeOptions.gwt.xml:12:
<inherits name='com.google.gwt.user.theme.standard.Standard'/>
Are you using the Standard styles? If not, you can remove this.

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7006#newcode13
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/DevModeOptions.gwt.xml:13:
<!-- <inherits name='com.google.gwt.user.theme.chrome.Chrome'/> -->
Should be able to remove these comments.

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

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7007#newcode36
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/client/DevModeOptions.java:36:
StyleInjector.inject(bundle.css().getText(), true);
Minor thing, but if you inject this in onModuleLoad the compiler will
not have to defensively call the static initializer on method
invocations.

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7013
File
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/client/resources/DevModeOptions.css
(right):

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7013#newcode1
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/client/resources/DevModeOptions.css:1:
@def TEXTWIDTH 30em;
Throw a copyright up here ^

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7013#newcode3
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/client/resources/DevModeOptions.css:3:
font-weight: bold;
I think your eclipse setup is putting either tabs or too many spaces for
the css indentation.

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7013#newcode17
plugins/npapi/DevModeOptions/src/com/google/gwt/devmodeoptions/client/resources/DevModeOptions.css:17:
color: IndianRed;
IndianRed! OMG, you're so unix!

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7015
File plugins/npapi/DevModeOptions/war/WEB-INF/web.xml (right):

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7015#newcode7
plugins/npapi/DevModeOptions/war/WEB-INF/web.xml:7: <!-- TODO: Add
<servlet> tags for each servlet here. -->
Use officially sanctioned TODO style: TODO(conroy):

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7016
File plugins/npapi/Makefile (left):

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7016#oldcode127
plugins/npapi/Makefile:127: ($(foreach src,$(SRCS),$(DEPEND)) true)
Makefile
Wow, I don't even know what this does. Next time I have Makefile
questions, I'm coming to you.

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7018
File plugins/npapi/ScriptableInstance.cpp (right):

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7018#newcode270
plugins/npapi/ScriptableInstance.cpp:270: NPN_GetProperty(getNPP(),
window, locationID, locationVariant.addressForReturn());
You should use NPN_GetValue with NPNVWindowNPObject to get the window
object and avoid using NPN_GetProperty. Technically, the window property
is const, but there shouldn't be any reason to rely on the property when
we can get the object we need directly from the context.

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7018#newcode338
plugins/npapi/ScriptableInstance.cpp:338: strncpy(out, retStr.c_str(),
retStr.size());
c_str is null terminated, so you should be able to safely copy size() +
1.

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7024
File plugins/npapi/prebuilt/gwt-dev-plugin/options.html (right):

http://gwt-code-reviews.appspot.com/1084801/diff/6001/7024#newcode1
plugins/npapi/prebuilt/gwt-dev-plugin/options.html:1: <!doctype html>
Do you even use this page?

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

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

Reply via email to