The patch did not apply cleanly. I went through and tried to provide
some feedback anyway.
http://gwt-code-reviews.appspot.com/893801/diff/16001/17001
File
user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonProxyListRequest.java
(right):
Reviewers: rjrjr,
Description:
Moved Property and EnumProperty from the
com.google.gwt.requestfactory.client.impl package to
com.google.gwt.requestfactory.shared.impl package.
Please review this at http://gwt-code-reviews.appspot.com/881801/show
Affected files:
M
Reviewers: rjrjr,
Description:
Fixed a bug in PropertyColumn introduced by r8780. PropertyColumn needs
access to the underlying class literals in order to get the value to
render.
Please review this at http://gwt-code-reviews.appspot.com/889801/show
Affected files:
M
Reviewers: rjrjr,
Description:
Fixes a couple of build breaks in the samples/expenses app.
Review by: rj...@google.com
Please review this at http://gwt-code-reviews.appspot.com/872801/show
Affected files:
M
http://gwt-code-reviews.appspot.com/842803/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/842803/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: amitmanjhi,
Description:
Fixes https://jira.springsource.org/browse/ROO-1213 - Hide Property from
public API.
Properties are now computed from the *Proxy classes and emitted into the
generated *ProxyImpl classes. Server side code will infer what the
properties should be by
Is there anything else left to do on this change? Alex's internship is
over, but we'd like to get the change in.
On 2010/08/10 08:44:14, zundel wrote:
Hi Andre, I'm waiting on Ray C or Lex to give the LGTM, but I noticed
this last patch you uploaded left out the editdistance library files.
On 2010/08/12 14:21:14, bobv wrote:
@Miguel,
FWIW, since we the time bounds of further optimizing this patch are
not yet
clear, I'd be in favor of landing it and then doing a second round
of
optimizations.
This patch achieves its stated goal, although there are some code
style issues
http://gwt-code-reviews.appspot.com/734801/diff/1/2
File user/src/com/google/gwt/junit/JUnitShell.java (right):
http://gwt-code-reviews.appspot.com/734801/diff/1/2#newcode1019
user/src/com/google/gwt/junit/JUnitShell.java:1019: + Try increasing
this timeout using the '-testMethodTimeout number'
LGTM
Let's circle back on this optimization that we have undone once we
stabilize.
http://gwt-code-reviews.appspot.com/457801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/328802/diff/1/2
File user/build.xml (right):
http://gwt-code-reviews.appspot.com/328802/diff/1/2#newcode161
user/build.xml:161: test.args=${test.web.remote.args} -out www -prod
-standardsMode -runStyle RemoteWeb:${gwt.hosts.web.remote}
Where are Xtries
LGTM
http://gwt-code-reviews.appspot.com/111802
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/112802/diff/1/2
File dev/core/src/com/google/gwt/dev/shell/OophmSessionHandler.java
(right):
http://gwt-code-reviews.appspot.com/112802/diff/1/2#newcode185
Line 185: moduleHandle.getLogger().log(TreeLogger.INFO,
Nit: Should this be Module XXX was loaded?
http://gwt-code-reviews.appspot.com/111801/diff/1/3
File dev/core/src/com/google/gwt/dev/DevModeBase.java (right):
http://gwt-code-reviews.appspot.com/111801/diff/1/3#newcode1053
Line 1053: for (String prenormalized : options.getStartupURLs()) {
If no startupURLs are specified on the command
how
to compute the URLs, but in that case GPE would not know how to compute
them either unless a user specified the -startupURLs on the command
line. The 90% case is local server so I'd think that GWT could compute
these.
On 2009/11/23 15:07:42, jat wrote:
On 2009/11/23 14:57:09, mmendez wrote
LGTM
http://gwt-code-reviews.appspot.com/111807/diff/1/5
File dev/core/src/com/google/gwt/dev/shell/remoteui/RemoteUI.java
(right):
http://gwt-code-reviews.appspot.com/111807/diff/1/5#newcode169
Line 169: public void setStartupUrls(MapString, URL urls) {
Nit: why was this a Map again?
LGTM
http://gwt-code-reviews.appspot.com/109801
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/103812/diff/5/1003
File dev/core/src/com/google/gwt/dev/DevMode.java (right):
http://gwt-code-reviews.appspot.com/103812/diff/5/1003#newcode234
Line 234: public static boolean isUsingRemoteUI() {
I agree that the code should be in DevModeBase and that we
LGTM with one nit. Consider removing the @param or @throws tags that
have no associated explanation or filling that explanation in.
http://gwt-code-reviews.appspot.com/105802
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: rdayal, jat,
Please review this at http://gwt-code-reviews.appspot.com/103810
Affected files:
M dev/core/src/com/google/gwt/dev/DevModeBase.java
M user/src/com/google/gwt/junit/JUnitShell.java
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: rdayal, jat,
Please review this at http://gwt-code-reviews.appspot.com/104810
Affected files:
M dev/core/src/com/google/gwt/dev/DevMode.java
M dev/core/src/com/google/gwt/dev/DevModeBase.java
M dev/core/src/com/google/gwt/dev/shell/BrowserListener.java
M
Thanks guys.
http://gwt-code-reviews.appspot.com/103810/diff/1/2
File dev/core/src/com/google/gwt/dev/DevModeBase.java (right):
http://gwt-code-reviews.appspot.com/103810/diff/1/2#newcode263
Line 263: private static final String CODE_SERVER_PORT_TAG =
-codeServerPort;
Actually, the name change
Thanks guys.
http://gwt-code-reviews.appspot.com/104810/diff/1/2
File dev/core/src/com/google/gwt/dev/DevMode.java (right):
http://gwt-code-reviews.appspot.com/104810/diff/1/2#newcode323
Line 323: Loading modules);
I can understand that, but is logging the best way to deal with this?
On
Reviewers: rdayal, jat,
Please review this at http://gwt-code-reviews.appspot.com/103813
Affected files:
M dev/core/src/com/google/gwt/dev/DevModeBase.java
Index: dev/core/src/com/google/gwt/dev/DevModeBase.java
diff --git a/dev/core/src/com/google/gwt/dev/DevModeBase.java
LGTM
Make sure to run the updates against the plugin as a cross-check.
http://gwt-code-reviews.appspot.com/102813
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: ,
Please review this at http://gwt-code-reviews.appspot.com/102803
Affected files:
M dev/core/src/com/google/gwt/dev/shell/remoteui/MessageTransport.java
M dev/core/src/com/google/gwt/dev/shell/remoteui/RemoteUI.java
M
Reviewers: jat,
Please review this at http://gwt-code-reviews.appspot.com/102805
Affected files:
M dev/core/src/com/google/gwt/dev/DevModeBase.java
Index: dev/core/src/com/google/gwt/dev/DevModeBase.java
diff --git a/dev/core/src/com/google/gwt/dev/DevModeBase.java
Thanks Rajeev! Also included Scott's assertion message improvements.
Committed as r6859.
http://gwt-code-reviews.appspot.com/102803/diff/1/3
File dev/core/src/com/google/gwt/dev/shell/remoteui/RemoteUI.java
(right):
http://gwt-code-reviews.appspot.com/102803/diff/1/3#newcode129
Line 129: +
LGTM
Included in r6859. Thanks Scott.
http://gwt-code-reviews.appspot.com/100805
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM
http://gwt-code-reviews.appspot.com/100801
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM - Just double check what state the system is in if you throw the
IllegalStateExceptions.
http://gwt-code-reviews.appspot.com/98801/diff/1/3
File dev/core/src/com/google/gwt/dev/shell/remoteui/RemoteUI.java
(right):
http://gwt-code-reviews.appspot.com/98801/diff/1/3#newcode119
Line 119:
LGTM - please run ant checkstyle before committing. Also, cherrypick
into the 2.0 branch.
On 2009/10/30 16:23:34, rdayal wrote:
http://gwt-code-reviews.appspot.com/89809
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
We used to be able to build typeoracles dynamically. If that is still
possible, you should add a unit test that generates twice and compares
the delta.
http://gwt-code-reviews.appspot.com/86806
--~--~-~--~~~---~--~~
LGTM
http://gwt-code-reviews.appspot.com/83809
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
I found a couple of failures on my OSX box last night. Let's look at
them together.
http://gwt-code-reviews.appspot.com/81805/diff/2003/2007
File
dev/core/test/com/google/gwt/dev/shell/remoteui/MessageTransportTest.java
(right):
LGTM
http://gwt-code-reviews.appspot.com/81805
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM
http://gwt-code-reviews.appspot.com/80805
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM
There are a couple of nits, but this does work end-to-end against the
custom plugin build.
http://gwt-code-reviews.appspot.com/83802/diff/1/2
File dev/core/src/com/google/gwt/dev/HostedModeBase.java (right):
http://gwt-code-reviews.appspot.com/83802/diff/1/2#newcode382
Line 382:
LGTM
http://gwt-code-reviews.appspot.com/83804
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM - Please make sure to add an issue to the public issue tracker so
that we don't forget to jarjar this. I've already verified that the
library works correctly after being rebased.
http://gwt-code-reviews.appspot.com/83803
--~--~-~--~~~---~--~~
Reviewers: rdayal,
Message:
Probably not. There is no hard convention around protoc generated
output that I know of.
On 2009/10/21 05:06:49, fabbott wrote:
http://gwt-code-reviews.appspot.com/83806/diff/1/2
File dev/build.xml (right):
Sorry that it took me a little while to review this code. I had to
refresh my memory on some of the JRE's built-in concurrency features.
That said, I think that I have two overall comments:
1) We should be able to simplify MessageProcessor if we reused some of
the JRE concurrency support.
2)
http://gwt-code-reviews.appspot.com/80805/diff/1/2
File
dev/core/src/com/google/gwt/dev/shell/remoteui/DevModeServiceServer.java
(right):
http://gwt-code-reviews.appspot.com/80805/diff/1/2#newcode55
Line 55: public void handleRequest(Request request) {
This queues a request, it doesn't
http://gwt-code-reviews.appspot.com/80806/diff/1/2
File
dev/core/src/com/google/gwt/dev/shell/remoteui/DevModeServiceServer.java
(right):
http://gwt-code-reviews.appspot.com/80806/diff/1/2#newcode117
Line 117: // TODO: There might be a better abstraction that one could
use here.
You could have
LGTM
http://gwt-code-reviews.appspot.com/77830
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
LGTM - I agree with Tamplin's comments on the package location. It's
been a while, but com.google.gwt.dev.shell.remoteui would seem like a
reasonable package location to me.
http://gwt-code-reviews.appspot.com/78825/diff/1/2
File dev/oophm/src/com/google/gwt/dev/shell/log/remotemessage.proto
Reviewers: Ray Ryan,
Description:
UiBinderGenerator is not stable and results extra compiles during web
app reload. In order to avoid unnecessary compiles and type oracle
refreshes, code generators must be stable.
This patch is a partial fix in that visitation orders have been
stabilized,
48 matches
Mail list logo
