http://gwt-code-reviews.appspot.com/954801/diff/1/6
File dev/core/src/com/google/gwt/dev/jjs/impl/OptimizerStats.java
(right):
http://gwt-code-reviews.appspot.com/954801/diff/1/6#newcode91
dev/core/src/com/google/gwt/dev/jjs/impl/OptimizerStats.java:91: *
Return a human readable string representing the values of all
statistics.
OK
On 2010/10/05 17:34:21, jat wrote:
human-readable
http://gwt-code-reviews.appspot.com/954801/diff/1/16
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditorWorkflow.java
(right):
http://gwt-code-reviews.appspot.com/954801/diff/1/16#newcode104
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditorWorkflow.java:104:
void onCancel(ClickEvent event) {
It's already checked.
On 2010/10/05 17:34:21, jat wrote:
Put the @SuppressWarnings on the parameter rather than the method,
here and
below.
However, I am not sure if we want to do this -- it seems pretty common
that
parameters on callbacks will be unused, and so I think it is better to
have
"Ignore in overriding and implementing methods" checked in the Eclipse
warnings
configuration.
http://gwt-code-reviews.appspot.com/954801/diff/1/19
File
samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseDetails.java
(left):
http://gwt-code-reviews.appspot.com/954801/diff/1/19#oldcode46
samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseDetails.java:46:
import com.google.gwt.requestfactory.shared.Request;
No idea :-(
On 2010/10/05 17:34:21, jat wrote:
Why wasn't this caught by ant checkstyle?
http://gwt-code-reviews.appspot.com/954801/diff/1/21
File
samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileReportEntry.java
(right):
http://gwt-code-reviews.appspot.com/954801/diff/1/21#newcode61
samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileReportEntry.java:61:
private final Listener listener;
I think it might be used by generated code but I'm not sure about that.
On 2010/10/05 17:34:21, jat wrote:
Why do we need the field at all if it is unused?
http://gwt-code-reviews.appspot.com/954801/diff/1/32
File user/src/com/google/gwt/app/place/Prefix.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/32#newcode27
user/src/com/google/gwt/app/place/Prefix.java:27: * {code
com.google.gwt.app.rebind.PlaceHistoryMapperGenerator} looks
Will link and include destination class
On 2010/10/05 17:34:21, jat wrote:
Why not link?
http://gwt-code-reviews.appspot.com/954801/diff/1/36
File user/src/com/google/gwt/editor/client/AutoBean.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/36#newcode54
user/src/com/google/gwt/editor/client/AutoBean.java:54: * Returns
<code>true</code> if {...@link #setFrozen} has been called.
Fixed
On 2010/10/05 17:34:21, jat wrote:
Shouldn't it talk about being last called with true? It looks like
the
freeze=>setFrozen change was to allow thawing a frozen object.
http://gwt-code-reviews.appspot.com/954801/diff/1/37
File user/src/com/google/gwt/editor/client/AutoBeanVisitor.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/37#newcode52
user/src/com/google/gwt/editor/client/AutoBeanVisitor.java:52: * TODO:
document.
Will do.
On 2010/10/05 17:34:21, jat wrote:
Should probably ask the person who wrote these methods to provide the
description.
http://gwt-code-reviews.appspot.com/954801/diff/1/44
File user/src/com/google/gwt/event/shared/EventBus.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/44#newcode27
user/src/com/google/gwt/event/shared/EventBus.java:27: * <p> See {...@code
com.google.gwt.event.shared.testing.CountingEventBus}.
I'll @link it and add the target to the javadoc build.
On 2010/10/05 17:34:21, jat wrote:
Why this change? And if you do want to move it into the main doc, why
not
@link?
http://gwt-code-reviews.appspot.com/954801/diff/1/50
File user/src/com/google/gwt/logging/client/HtmlLogFormatter.java
(right):
http://gwt-code-reviews.appspot.com/954801/diff/1/50#newcode62
user/src/com/google/gwt/logging/client/HtmlLogFormatter.java:62: *
@param event
My scheme is more or less as follows:
If the unused param occurs in 'frameworky' code, where we expect there
to be subclasses that make use of the param, javadoc it
If the method occurs in code that is unlikely to be further subclassed
(but isn't one of the situations where checking the box in Eclipse will
remove the warning, such as can happen with UiBinder), use
@SuppressWarnings
On 2010/10/05 17:34:21, jat wrote:
Why use this method here and @SuppressWarnings elsewhere? Again, I am
not sure
we want to make these changes rather than just change Eclipse's
warnings
settings (I believe what you get by following eclipse/README.txt will
not warn
on these).
http://gwt-code-reviews.appspot.com/954801/diff/1/65
File user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java
(right):
http://gwt-code-reviews.appspot.com/954801/diff/1/65#newcode31
user/src/com/google/gwt/safehtml/client/SafeHtmlTemplates.java:31: *
&064;Template("<span class=\"{3}\">{0}: <a href=\"{1}\">{2}</a></span>")
Fixed
On 2010/10/05 17:34:21, jat wrote:
Isn't this supposed to be "@"?
http://gwt-code-reviews.appspot.com/954801/diff/1/68
File user/src/com/google/gwt/uibinder/client/UiChild.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/68#newcode43
user/src/com/google/gwt/uibinder/client/UiChild.java:43: * &064;UiChild
MyWidget#addCustomChild(Widget w) </code> and
Fixed
On 2010/10/05 17:34:21, jat wrote:
And here.
http://gwt-code-reviews.appspot.com/954801/diff/1/72
File user/src/com/google/gwt/user/client/ResponseTextHandler.java
(right):
http://gwt-code-reviews.appspot.com/954801/diff/1/72#newcode18
user/src/com/google/gwt/user/client/ResponseTextHandler.java:18: // TODO
- is this class still used anywhere?
It has users in Google3, maybe best to remove it in a separate CL.
On 2010/10/05 17:34:21, jat wrote:
It doesn't appear to be -- shouldn't we go ahead and remove it?
http://gwt-code-reviews.appspot.com/954801/diff/1/75
File
user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java
(right):
http://gwt-code-reviews.appspot.com/954801/diff/1/75#newcode276
user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java:276:
@SuppressWarnings("unused")
Done
On 2010/10/05 17:34:21, jat wrote:
Why not just rewrite this as:
if (statsContext.isStatsAvailable()) {
statsContext.stats(statsContext.bytesStat(methodName,
requestData.length(), "requestSent");
}
http://gwt-code-reviews.appspot.com/954801/diff/1/80
File user/src/com/google/gwt/user/client/ui/Label.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/80#newcode408
user/src/com/google/gwt/user/client/ui/Label.java:408: * described in
{...@link #setText(String,
com.google.gwt.i18n.client.HasDirection.Direction)
Fixed
On 2010/10/05 17:34:21, jat wrote:
Line too long.
http://gwt-code-reviews.appspot.com/954801/diff/1/86
File user/src/com/google/gwt/user/client/ui/TabPanel.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/86#newcode70
user/src/com/google/gwt/user/client/ui/TabPanel.java:70: //Cannot do
anything about tab panel implementing TabListener until next release.
Fixed
On 2010/10/05 17:34:21, jat wrote:
Space after //, line too long.
http://gwt-code-reviews.appspot.com/954801/diff/1/88
File user/src/com/google/gwt/user/client/ui/Widget.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/88#newcode381
user/src/com/google/gwt/user/client/ui/Widget.java:381: * {...@link
com.google.gwt.event.logical.shared.AttachEvent.Handler
AttachEvent.Handler}s.
Fixed
On 2010/10/05 17:34:21, jat wrote:
Line too long.
http://gwt-code-reviews.appspot.com/954801/diff/1/88#newcode390
user/src/com/google/gwt/user/client/ui/Widget.java:390: * {...@link
com.google.gwt.event.logical.shared.AttachEvent.Handler
AttachEvent.Handler}s.
Fixed
On 2010/10/05 17:34:21, jat wrote:
Line too long.
http://gwt-code-reviews.appspot.com/954801/diff/1/92
File user/src/com/google/gwt/xhr/client/XMLHttpRequest.java (right):
http://gwt-code-reviews.appspot.com/954801/diff/1/92#newcode26
user/src/com/google/gwt/xhr/client/XMLHttpRequest.java:26: *
XMLHttpRequest/</a>/
Fixed
On 2010/10/05 17:34:21, jat wrote:
Won't this line break add a space in the displayed URL? Probably
better to do
it as
...est/"
>http://...
which will avoid that.
Likewise all the ones below.
http://gwt-code-reviews.appspot.com/954801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
