http://gwt-code-reviews.appspot.com/1828803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1829803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/09/14 14:37:01, rdayal wrote:
Pair-reviewed by drfibonacci and rdayal.
LGTM.
http://gwt-code-reviews.appspot.com/1828803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1829803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/09/14 18:53:22, rdayal wrote:
I think this one is going to be pushed past GWT 2.5; that's ok though.
http://gwt-code-reviews.appspot.com/1829803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1828803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/09/12 22:50:12, rdayal wrote:
Ping...I'd like to get this into 2.5.
http://gwt-code-reviews.appspot.com/1829803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: tbroyer, drfibonacci,
Description:
Update Maven sample pom.xml files to use maven-compiler-plugin's
annotation processing functionality. It is now understood by m2e-apt, so
we can get rid of the hacks we had to put in to make this work in
Eclipse/GPE. This also gets rid of the nasty
Reviewers: tbroyer, drfibonacci, rchandia,
Description:
Move GAE Auth functionality from Expenses over the MobileWebApp sample.
Please review this at http://gwt-code-reviews.appspot.com/1829803/
Affected files:
M samples/dynatablerf/README-MAVEN.txt
M samples/dynatablerf/pom.xml
M
On 2012/09/11 20:36:11, rdayal wrote:
Had to re-create this; my client was messed up. Original review was
here:
http://gwt-code-reviews.appspot.com/1806803/
Note that this review is built on the changes here:
http://gwt-code-reviews.appspot.com/1828803/
(hence they are included)
http://gwt
Sorry for the delay.
LGTM.
http://gwt-code-reviews.appspot.com/1794803/diff/4001/user/src/com/google/gwt/uibinder/rebind/FieldManager.java
File user/src/com/google/gwt/uibinder/rebind/FieldManager.java (right):
It looks good to me, but isn't this based on CLDR data? If so, this
change may get wiped out when we update CLDR again.
Adding jat and shanjian for comment.
http://gwt-code-reviews.appspot.com/1798805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: drfibonacci, tbroyer,
Description:
Update Maven sample pom.xml files to use maven-compiler-plugin's
annotation processing functionality. It is now understood by m2e-apt, so
we can get rid of the hacks we had to put in to make this work in
Eclipse/GPE. This also gets rid of the nasty
http://gwt-code-reviews.appspot.com/1804803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
From pair review (rchandia, rdayal):
Let's fix up these things below, and go one more round.
http://gwt-code-reviews.appspot.com/1794803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderParser.java
File user/src/com/google/gwt/uibinder/rebind/UiBinderParser.java
(right):
http://gwt-code
On 2012/08/01 12:06:09, Patrick Tucker wrote:
On 2012/07/31 18:48:57, skybrian wrote:
LGTM
Committed in r11189.
http://gwt-code-reviews.appspot.com/1801803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/07/31 18:13:13, skybrian wrote:
LGTM
Committed in r11188.
http://gwt-code-reviews.appspot.com/1800803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks for the review.
http://gwt-code-reviews.appspot.com/1800803/diff/1/user/test/com/google/gwt/core/public/script_injector_test_absolute.js
File
user/test/com/google/gwt/core/public/script_injector_test_absolute.js
(right):
Reviewers: Patrick Tucker, skybrian,
Description:
Fix incorrect SuggestBox documentation related to adding selection
handlers. Fixes issue 4575.
Please review this at http://gwt-code-reviews.appspot.com/1801803/
Affected files:
M user/src/com/google/gwt/user/client/ui/SuggestBox.java
On 2012/07/04 21:47:12, kurka.daniel wrote:
Simply typo in javadoc needs review
LGTM.
http://gwt-code-reviews.appspot.com/1770803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Hey all,
We're excited to announce the GWT 2.5 Release Candidate! Read about it on
the GWT Blog http://googlewebtoolkit.blogspot.com/2012/06/gwt-2.html, and
download it here http://developers.google.com/web-toolkit/download (bottom
of the page). The RC has been uploaded to Maven Central with
LGTM, but take a look at the comments before committing.
http://gwt-code-reviews.appspot.com/1758804/diff/2001/distro-source/core/src/samples/build.xml
File distro-source/core/src/samples/build.xml (right):
On 2012/06/25 20:52:50, rchandia wrote:
LGTM.
http://gwt-code-reviews.appspot.com/1758804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM.
http://gwt-code-reviews.appspot.com/1759803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: tbroyer, skybrian,
Description:
Fix requestfactory test errors, and compilation failures in
dev/compile.test resulting from the unbund
ling of streamhtmlparser.
Please review this at http://gwt-code-reviews.appspot.com/1743808/
Affected files:
M dev/build.xml
M
Reviewers: skybrian,
Description:
Increase heap size for testing to prevent GC thrashing and OOMs when
running usr/BigIntegerSuite.
Please review this at http://gwt-code-reviews.appspot.com/1750803/
Affected files:
M common.ant.xml
Index: common.ant.xml
http://gwt-code-reviews.appspot.com/1750803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: jat,
Description:
Move test Messages files from client to shared; was missed in r10061.
This was causing I18NSuite to fail.
Please review this at http://gwt-code-reviews.appspot.com/1752803/
Affected files:
D
On 2012/06/15 02:39:33, skybrian wrote:
I verified that the ant build and google builds both work. The ant
build takes
about 9 minutes with this change, versus about 5:30 before. Also,
elemental gets
a full rebuild each time so a no-op build now takes about 4:30.
Thanks for checking this
https://gwt-code-reviews.appspot.com/1727808/diff/16001/distro-source/build.xml
File distro-source/build.xml (right):
https://gwt-code-reviews.appspot.com/1727808/diff/16001/distro-source/build.xml#newcode39
distro-source/build.xml:39: zipfileset
file=${gwt.build.lib}/gwt-elemental.jar
=${gwt.build.lib}/gwt-elemental.jar prefix=${project.distname}
/
On 2012/06/15 15:39:27, rdayal wrote:
Will this cause the build to fail if gwt-elemental.jar is not
present?
No.
If you use ant -quiet clean dist, you'll see the Building SDK
without
Elemental warning from above
LGTM.
This reminds me that when we start doing JDK7 builds of GWT, we should
set -target 1.6, so that GWT can still run on JDK 1.6.
http://gwt-code-reviews.appspot.com/1743803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM.
http://gwt-code-reviews.appspot.com/1743804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: skybrian,
Description:
Prevent compilation failures due to type inferencing bugs (bug 6302954
at bugs.sun.com) in older versions of JDK6.
Please review this at http://gwt-code-reviews.appspot.com/1736805/
Affected files:
M user/src/com/google/gwt/core/client/GWT.java
M
On 2012/05/25 23:01:27, jat wrote:
LGTM
Committed as r11044.
https://gwt-code-reviews.appspot.com/1716804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/06/08 19:39:13, rdayal wrote:
On 2012/05/22 16:11:49, tbroyer wrote:
LGTM.
Committed a r11045.
https://gwt-code-reviews.appspot.com/1713803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM.
http://gwt-code-reviews.appspot.com/1734804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM with a small nit and recommendation.
http://gwt-code-reviews.appspot.com/1730805/diff/1/user/test/com/google/gwt/editor/client/DirtyEditorTest.java
File user/test/com/google/gwt/editor/client/DirtyEditorTest.java
(right):
What are your thoughts on this getting into 2.5 vs. 2.5.1? I'd like to
get it into 2.5, but I'm a bit worried that we may not come to consensus
in time..
http://gwt-code-reviews.appspot.com/1619803/diff/7001/user/src/com/google/gwt/user/client/ui/ValueListBox.java
File
lgtm.
http://gwt-code-reviews.appspot.com/1728805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM.
http://gwt-code-reviews.appspot.com/1733805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/05/31 16:11:23, stephenh wrote:
Ping Ping.
Thanks--added a new patchset with a performance note in the javadocs.
Committed as r11032.
http://gwt-code-reviews.appspot.com/1614806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/05/22 16:11:49, tbroyer wrote:
LGTM.
https://gwt-code-reviews.appspot.com/1713803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
/UiBinderWriter.java#newcode1138
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1138:
private void ensureInjectedCssFields() {
On 2012/06/01 23:09:27, rchandia wrote:
On 2012/05/27 19:26:12, rdayal wrote:
thought bubble
I wonder if this leads to code bloat (i.e. having to litter
https://gwt-code-reviews.appspot.com/1646803/diff/21002/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
File
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(right):
at
least,
response.length() = 2);
On 2012/05/31 14:24:35, rdayal wrote:
you mean size(), not length(), right? :)
Oh my! Yes, of course.
I've run ant clean testrf to make sure it's all OK.
Committed as r11003.
https://gwt-code-reviews.appspot.com/1646803/
--
http://groups.google.com/group/Google
Double-ping. If we can't land this one soon, we'll defer it to 2.5.1.
http://gwt-code-reviews.appspot.com/1578808/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/05/22 23:28:12, rdayal wrote:
Ping.
Ping Ping.
http://gwt-code-reviews.appspot.com/1614806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/05/14 19:34:17, rdayal wrote:
On 2012/04/10 21:00:17, acleung wrote:
LGTM
Alan, did this one land internally?
This was submitted.
http://gwt-code-reviews.appspot.com/1647803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/05/30 06:16:52, stephenh wrote:
I'll submit your change after the API Checker config file is
updated. We
should
just need to add these lines to it:
java.lang.Boolean::FALSE FINAL_ADDED
java.lang.Boolean::TRUE FINAL_ADDED
Great, that sounds good to me. Thanks, John.
Thanks for
LGTM. Looks like we're missing the deletion of the 23_24 config file?
http://gwt-code-reviews.appspot.com/1721803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM.
https://gwt-code-reviews.appspot.com/1646803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
https://gwt-code-reviews.appspot.com/1646803/diff/14003/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
File
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(right):
Hey Stephen - I'll do some digging and see what I can find out. Do you
mind posting your updated config file as part of the patch?
http://gwt-code-reviews.appspot.com/1710804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1622803/diff/6001/user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java
File
user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java
(right):
/requestfactory/server/Resolver.java#newcode365
user/src/com/google/web/bindery/requestfactory/server/Resolver.java:365:
new IdentityHashMapBaseProxy, Resolution();
On 2012/05/27 08:41:35, tbroyer wrote:
On 2012/05/23 01:08:29, rdayal wrote:
Sorry for the long delay on this, but doing a bit of thinking
/requestfactory/server/Resolver.java#newcode365
user/src/com/google/web/bindery/requestfactory/server/Resolver.java:365:
new IdentityHashMapBaseProxy, Resolution();
On 2012/05/29 17:08:02, rdayal wrote:
On 2012/05/27 08:41:35, tbroyer wrote:
On 2012/05/23 01:08:29, rdayal wrote:
Sorry for the long delay
#newcode1576
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1576: if
(field == null || (!FieldWriterType.DEFAULT.equals(field.getFieldType())
On 2012/05/24 14:49:24, rchandia wrote:
On 2012/05/23 01:51:11, rdayal wrote:
Maybe wrap this small block of code into a helper method
://gwt-code-reviews.appspot.com/1601806/diff/40003/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode706
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:706:
boolean isDiffing() {
On 2012/05/22 23:03:44, rdayal wrote
LGTM.
http://gwt-code-reviews.appspot.com/1703803/diff/1/user/src/com/google/gwt/user/client/ui/AbsolutePanel.java
File user/src/com/google/gwt/user/client/ui/AbsolutePanel.java (right):
Stephen, do you want to open a new issue for this (with a clean patch)?
http://gwt-code-reviews.appspot.com/1528806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/05/23 16:20:30, stephenh wrote:
Ran this through our battery of tests, and this is the failure that I
saw:
apicheck-nobuild:
[java] Found 38 new resources
[java] Found 38 new resources
[java] ApiChanges [
[java]
Just a minor nit; otherwise LGTM.
http://gwt-code-reviews.appspot.com/1601806/diff/40003/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
File
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
(right):
LGTM. Going to run this back through the battery of tests.
http://gwt-code-reviews.appspot.com/1622803/diff/6001/user/test/com/google/web/bindery/requestfactory/server/SimpleFoo.java
File
user/test/com/google/web/bindery/requestfactory/server/SimpleFoo.java
(left):
Ping.
http://gwt-code-reviews.appspot.com/1614806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Passes the internal set of tests now. Yay! Skybrian's going to do the
internal review.
http://gwt-code-reviews.appspot.com/1622803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
/requestfactory/server/Resolver.java#newcode365
user/src/com/google/web/bindery/requestfactory/server/Resolver.java:365:
new IdentityHashMapBaseProxy, Resolution();
On 2012/04/10 08:05:33, tbroyer wrote:
On 2012/04/09 15:44:43, rdayal wrote:
Looks good, but I must say that I dont' quite understand how/why
Ping (ray).
http://gwt-code-reviews.appspot.com/1618807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Great job, and thanks for doing this. Let's go one more round.
Also, would you be willing to update the UIBinder docs? :D :D
http://gwt-code-reviews.appspot.com/1700803/diff/1/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
File
Brian - thanks for the detailed questions. Your questions, and Thomas'
replies, have made me understand RequestFactory much better than I did
before. I was also curious about why EntityProxies had associations with
RequestContexts in some cases but not others.
On 2012/05/17 12:59:25, tbroyer
Woohoo! Finally, everything passes!
http://gwt-code-reviews.appspot.com/1601806/diff/35001/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
File
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
(right):
Had to roll this back. These are the test failures that I'm seeing:
1)
testValueObjectCreateSetRetrieveUpdateViaList(com.google.web.bindery.requestfactory.gwt.client.RequestFactoryTest)java.lang.RuntimeException:
Remote test failed at 127.0.0.1 / Mozilla/5.0 (X11; U; Linux i686
(x86_64); en-US;
Hey guys,
Looks like this one has been hanging out here for a long time - how
strongly do you guys feel about this getting into 2.5?
http://gwt-code-reviews.appspot.com/1601804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Hey guys,
Looks like this one has been hanging out here for a long time - how
strongly do you guys feel about this getting into 2.5?
http://gwt-code-reviews.appspot.com/1601804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Hey guys,
Looks like this one has been hanging out here for a long time - how
strongly do you guys feel about this getting into 2.5?
Rajeev
http://gwt-code-reviews.appspot.com/1601804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/04/10 21:00:17, acleung wrote:
LGTM
Alan, did this one land internally?
http://gwt-code-reviews.appspot.com/1647803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/04/09 18:55:16, scottb wrote:
Stephen, sorry for the extremely late reply, Rajeev is in the process
of getting
this landed.
http://gwt-code-reviews.appspot.com/1578808/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File
Thanks for doing this patch, Stephen.
What would you say the priority is for 2.5? Is this breaking/preventing
something in the Scala-GWT project?
http://gwt-code-reviews.appspot.com/1528806/diff/1009/user/super/com/google/gwt/emul/java/lang/Character.java
File
Ok, the original failure we were looking at is gone! Yay! Now, two more
to deal with:
1)
testChangedEdit(com.google.web.bindery.requestfactory.gwt.client.RequestFactoryTest)java.lang.RuntimeException:
Remote test failed at 127.0.0.1 / Mozilla/5.0 (X11; U; Linux i686
(x86_64); en-US; rv:1.9.1.2)
On 2012/04/09 18:33:00, rdayal wrote:
On 2012/02/28 05:16:42, stephenh wrote:
instance in the sense of instance method
Done.
submitted.
We had to roll this back because it broke a bunch of test code in the
Google environment. I think that this patch is in the right, but the
test code
Was debating whether this one should go in for GWT 2.5, but my
inclination is to skip it. Please let me know if anyone feels strongly
about this.
http://gwt-code-reviews.appspot.com/1380807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/04/05 19:48:18, zundel wrote:
Ping Ping.
http://gwt-code-reviews.appspot.com/1513803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2011/10/09 13:58:44, Lex Spoon wrote:
For what it's worth, this patch does not seem necessary for the
Scala+GWT
project. It should be evaluated on its own merits.
Stephen, at a high level, what is this patch for? How important is it
for GWT 2.5?
On 2011/10/13 00:25:02, stephenh wrote:
This is just a spike, but client-side code now gets successfully
recompiled if
its binary-only annotations go-away/appear/change.
A few notes:
All of the getResourceAsStream calls (especially for the made up
simple/qualified refs that come out of
/04/09 15:44:43, rdayal wrote:
Looks good, but I must say that I dont' quite understand how/why
this fixes
the
problem. Can you point out how the code execution in this class
would have
resulted in a breakage with a HashMap vs. an IdentityHashMap?
At line 632, a collection is populated
, tbroyer wrote:
On 2012/04/09 15:12:50, rdayal wrote:
I'm an autobean newbie, but shouldn't we change the diffing
algorithm so that
it
doesn't actually cause edits to proxies? I mean, when you a do a
diff, that
should never happen, right?
The problem is that autobean has no notion of read
Hey Thomas,
When I ran the tests after applying your change, I saw a failure in
EditorTest.test:
aused by: junit.framework.AssertionFailedError: expected=EditorBarTest
actual=FOO
at
com.google.web.bindery.requestfactory.gwt.client.ui.EditorTest.onSuccess(EditorTest.java:164)
at
Alan, can you review this one?
http://gwt-code-reviews.appspot.com/1647803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/01/23 23:43:12, rdayal wrote:
LGTM.
Submitted.
http://gwt-code-reviews.appspot.com/1588803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Generally looks good, just a couple of suggestions. Let's go one more
round on this.
http://gwt-code-reviews.appspot.com/1601806/diff/1015/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
File
Generally, looks good, but wouldn't mind hearing your comments to my
questions before the submit.
https://gwt-code-reviews.appspot.com/1646803/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java
File user/src/com/google/web/bindery/requestfactory/server/Resolver.java
LGTM.
http://gwt-code-reviews.appspot.com/1622803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/03/04 13:02:42, tbroyer wrote:
LGTM.
https://gwt-code-reviews.appspot.com/1653803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/02/28 05:16:42, stephenh wrote:
instance in the sense of instance method
Done.
submitted.
http://gwt-code-reviews.appspot.com/1620805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2011/12/19 20:25:20, scottb wrote:
LGTM
Submitted.
http://gwt-code-reviews.appspot.com/1614805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Looks good to me, but I'd like John to take a look.
http://gwt-code-reviews.appspot.com/1573803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM.
http://gwt-code-reviews.appspot.com/1580804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/04/05 21:55:42, rdayal wrote:
LGTM.
Submitted.
http://gwt-code-reviews.appspot.com/1580804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Submitted.
http://gwt-code-reviews.appspot.com/1582803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/04/05 18:59:13, rdayal wrote:
Jason, can you look at this one more time? Sorry about the delay. Had to
make a couple of changes:
1) You can't map null values in a ConcurrentHashMap. Bad suggestion on
my part.
2) It's a change in behavior to pass null into readContent. Changed
1 - 100 of 228 matches
Mail list logo