[gwt-contrib] Re: This patch makes three changes: (issue1093801)
On Tue, Nov 9, 2010 at 12:32 PM, j...@google.com wrote: http://gwt-code-reviews.appspot.com/1093801/diff/1/2 File tools/api-checker/config/gwt20_21userApi.conf (right): http://gwt-code-reviews.appspot.com/1093801/diff/1/2#newcode99 tools/api-checker/config/gwt20_21userApi.conf:99: :com.google.gwt.core.client.impl\ This came up in a separate CL -- should we just exclude *.impl.* ? It seems like we would never want to consider changes to impl classes as API breakage. I think this came up in the past as well. My vote is still for listing them explicitly since the impl thing is a GWT convention, and ideally, we should try to avoid api breakages in any public api even if it is in the impl package. However, if someone does want to exclude particular impl packages, adding an entry is easy. http://gwt-code-reviews.appspot.com/1093801/diff/1/3 File tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityUnitTest.java (right): http://gwt-code-reviews.appspot.com/1093801/diff/1/3#newcode110 tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityUnitTest.java:110: * from the most specific to the least specific. So if Foo(Object...) is present, Foo(null) would always resolve to Foo(String...)? yes. http://gwt-code-reviews.appspot.com/1093801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Ensure that all base URL's are absolut-ify-ed, no matter where we get them from. (issue1066801)
LGTM On Fri, Oct 29, 2010 at 5:41 PM, unn...@google.com wrote: http://gwt-code-reviews.appspot.com/1066801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove the onFailure call in case of unauthenticated user. This gets rid of the (issue1061801)
Perhaps for non-roo apps, there could be an alternative AuthenticationFailureHandler which simply displays a pop-up user not logged in. On Thu, Oct 28, 2010 at 3:49 PM, BobV b...@google.com wrote: On Thu, Oct 28, 2010 at 6:26 PM, Unnur Gretarsdottir unn...@google.com wrote: Bob - why is the event listener not installed by default in non roo apps? I think it was when I originally added this code (although it has been a while) There is no good default behavior in this case; authentication almost seems to define the phrase site-specific. The implementation of AuthenticationFailureHandler makes assumptions about the serving environment, namely that the login and userId headers will be defined. If the login header is undefined, Location.replace() is called with a null value. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixed the serialization bug that the version numbers were not being sent back (issue930802)
Submitted http://code.google.com/p/google-web-toolkit/source/detail?r=8932 On Mon, Oct 4, 2010 at 5:59 PM, rj...@google.com wrote: LGTM http://gwt-code-reviews.appspot.com/930802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)
The updated patch incorporates the feedback. On Tue, Oct 5, 2010 at 11:59 AM, amitman...@google.com wrote: http://gwt-code-reviews.appspot.com/959801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)
On Tue, Oct 5, 2010 at 1:26 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/959801/diff/4001/5001 File user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (right): http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode78 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:78: this.version = givenVersion; version = jsonObject.has(Constants.ENCODED_VERSION_PROPERTY) ? jsonObject.getString(Constants.ENCODED_VERSION_PROPERTY) : null; For that matter, do you even need the has() call? Will getString return null if it isn't set? If the property is absent, getString(..) returns an JSONException. http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1371 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1371: // handle DELETE What are you fixing by getting rid of the write operation check? The objective was not to get rid of the write operation check, but just to refactor the code so that the flow is cleaner. After refactoring, a boolean was all that was necessary. http://gwt-code-reviews.appspot.com/959801/diff/4001/5002 File user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java (right): http://gwt-code-reviews.appspot.com/959801/diff/4001/5002#newcode150 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java:150: public void testEndToEndSmartDiff_NoChange() throws Exception { It looks like you're testing the version code path, but no longer testing the non-version code path? Until we delete the latter, we should maintain its coverage. Fixed in the followup patch. http://gwt-code-reviews.appspot.com/959801/diff/4001/5003 File user/test/com/google/gwt/requestfactory/server/SimpleFoo.java (right): http://gwt-code-reviews.appspot.com/959801/diff/4001/5003#newcode200 user/test/com/google/gwt/requestfactory/server/SimpleFoo.java:200: s1.isChanged = false; This is pointless, it defaults to false. http://gwt-code-reviews.appspot.com/959801/diff/4001/5003#newcode206 user/test/com/google/gwt/requestfactory/server/SimpleFoo.java:206: s2.isChanged = false; ditto Done. http://gwt-code-reviews.appspot.com/959801/diff/4001/5003#newcode283 user/test/com/google/gwt/requestfactory/server/SimpleFoo.java:283: * increment version numbers. Won't be able to use autobean, this is a JRE class. Wasn't thinking. Document that it is only set by setUserName and the other one. Done. http://gwt-code-reviews.appspot.com/959801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)
Done everything. On Tue, Oct 5, 2010 at 1:32 PM, b...@google.com wrote: http://gwt-code-reviews.appspot.com/959801/diff/4001/5001 File user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (right): http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1381 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1381: * send an UPDATE if the client is at a version different than that of the s/s/S/ http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1382 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1382: * server or if the server version has changed. or if the server version has changed after invoking the domain method. http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1385 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1385: assert entityInstanceAfterOperation != null; Unnecessary assert. http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode1445 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:1445: SerializedEntity beforeEntity = beforeDataMap.get(entityKey); What would a null beforeEntity imply? That the key was just created on the server? Yes, it would imply that the beforeEntity was just created on the server. http://gwt-code-reviews.appspot.com/959801/diff/4001/5002 File user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java (right): http://gwt-code-reviews.appspot.com/959801/diff/4001/5002#newcode150 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java:150: public void testEndToEndSmartDiff_NoChange() throws Exception { Install eclemma and verify coverage before submitting. http://gwt-code-reviews.appspot.com/959801/diff/4001/5003 File user/test/com/google/gwt/requestfactory/server/SimpleFoo.java (right): http://gwt-code-reviews.appspot.com/959801/diff/4001/5003#newcode283 user/test/com/google/gwt/requestfactory/server/SimpleFoo.java:283: * increment version numbers. No plans to implement a server version of AutoBeans at present. http://gwt-code-reviews.appspot.com/959801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)
Updated patch incorporates the feedback, including adding the additional test. On Tue, Oct 5, 2010 at 2:14 PM, amitman...@google.com wrote: http://gwt-code-reviews.appspot.com/959801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)
On Tue, Oct 5, 2010 at 2:21 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/959801/diff/4001/5001 File user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (right): http://gwt-code-reviews.appspot.com/959801/diff/4001/5001#newcode78 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:78: this.version = givenVersion; Not done. Finally done. http://gwt-code-reviews.appspot.com/959801/diff/12002/13002 File user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java (right): http://gwt-code-reviews.appspot.com/959801/diff/12002/13002#newcode199 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java:199: public void testEndToEndSmartDiff_NoChange_NoVersion() throws Exception { If I read this right, you're testing a config that has nothing to do with reality — the domain object has a version number, but the browser has dropped it. The interesting test is of a domain object that has no version number, or at least no version change. Since you have properties that don't tickle the version, that shouldn't be hard to write. Am I confused? The test above simulated a case where the domain object has no version number. But as you note above, there could be a scenario where the domain object has a version number, but the client version and the server version is the same. Added another test for that. http://gwt-code-reviews.appspot.com/959801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Send an update notification to a stale client by comparing the incoming version (issue959801)
On Tue, Oct 5, 2010 at 4:10 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/959801/diff/5004/20001 File user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (right): http://gwt-code-reviews.appspot.com/959801/diff/5004/20001#newcode77 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:77: this.version = (Integer) (jsonObject.has(Constants.ENCODED_VERSION_PROPERTY) Why are you hard coding this requirement that the thing be an integer? The version property is currently hard coded to be an Integer. I'm just using that here. http://gwt-code-reviews.appspot.com/959801/diff/5004/20002 File user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java (right): http://gwt-code-reviews.appspot.com/959801/diff/5004/20002#newcode200 user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java:200: public void testEndToEndSmartDiff_NoChange_NoVersion() throws Exception { Why keep the test that tests surviving what would be a client bug? http://gwt-code-reviews.appspot.com/959801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes to use Java 1.5 compatible JARs (issue939802)
LGTM On Mon, Oct 4, 2010 at 3:11 PM, rchan...@google.com wrote: Reviewers: amitmanjhi, Description: Fixes to use Java 1.5 compatible JARs Fixed DynaTableRf eclipse .classpath to use json-1.5.jar Fixed build scripts and eclipse classpaths to use java 1.5 compatible StreamHtmlParser JAR Review by: amitman...@google.com Please review this at http://gwt-code-reviews.appspot.com/939802/show Affected files: M dev/build.xml M eclipse/samples/DynaTableRf/.classpath M eclipse/user/.classpath Index: dev/build.xml === --- dev/build.xml (revision 8929) +++ dev/build.xml (working copy) @@ -100,7 +100,7 @@ include name=htmlunit/htmlunit-r5940/htmlunit-r5940.jar / include name=htmlunit/htmlunit-r5940/htmlunit-core-js-r5940.jar / include name=nekohtml/nekohtml-1.9.13.jar / - include name=streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-rebased.jar / + include name=streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-1.5-rebased.jar / include name=xalan/xalan-2.7.1.jar / include name=xerces/xerces-2_9_1/serializer.jar / include name=xerces/xerces-2_9_1/xercesImpl-NoMetaInf.jar / @@ -124,7 +124,7 @@ zipfileset src=${gwt.tools.lib}/jetty/jetty-6.1.11.jar / zipfileset src=${gwt.tools.lib}/icu4j/icu4j-4_4_1.jar / zipfileset src=${gwt.tools.lib}/protobuf/protobuf-2.2.0/protobuf-java-rebased-2.2.0.jar / - zipfileset src=${gwt.tools.lib}/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-rebased.jar / + zipfileset src=${gwt.tools.lib}/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-1.5-rebased.jar / zipfileset src=${gwt.tools.lib}/tomcat/ant-launcher-1.6.5.jar / zipfileset src=${gwt.tools.lib}/tomcat/catalina-1.0.jar / zipfileset src=${gwt.tools.lib}/tomcat/catalina-optional-1.0.jar / Index: eclipse/samples/DynaTableRf/.classpath === --- eclipse/samples/DynaTableRf/.classpath (revision 8929) +++ eclipse/samples/DynaTableRf/.classpath (working copy) @@ -6,6 +6,6 @@ classpathentry combineaccessrules=false kind=src path=/gwt-user/ classpathentry kind=var path=GWT_TOOLS/lib/javax/validation/validation-api-1.0.0.GA.jar sourcepath=/GWT_TOOLS/lib/javax/validation/validation-api-1.0.0.GA-sources.jar/ classpathentry kind=var path=GWT_TOOLS/lib/javax/validation/validation-api-1.0.0.GA-sources.jar/ - classpathentry kind=var path=GWT_TOOLS/redist/json/r2_20080312/json.jar/ + classpathentry kind=var path=GWT_TOOLS/redist/json/r2_20080312/json-1.5.jar/ classpathentry kind=output path=war/WEB-INF/classes/ /classpath Index: eclipse/user/.classpath === --- eclipse/user/.classpath (revision 8929) +++ eclipse/user/.classpath (working copy) @@ -40,6 +40,6 @@ classpathentry exported=true kind=var path=GWT_TOOLS/lib/javax/validation/validation-api-1.0.0.GA-sources.jar/ classpathentry kind=var path=GWT_TOOLS/lib/jetty/jetty-6.1.11.jar sourcepath=/GWT_TOOLS/lib/jetty/jetty-6.1.11-src.zip/ classpathentry kind=var path=GWT_TOOLS/lib/guava/guava-r06/guava-r06-rebased.jar/ - classpathentry kind=var path=GWT_TOOLS/lib/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-rebased.jar/ + classpathentry kind=var path=GWT_TOOLS/lib/streamhtmlparser/streamhtmlparser-jsilver-r10/streamhtmlparser-jsilver-r10-1.5-rebased.jar/ classpathentry kind=output path=bin/ /classpath -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RR : Overhaul the client-side RequestFactory code (issue924801)
LGTM. Other than the comments on wave that you have already looked at, I don't have anything new. I carefully looked at the EntityProxy and stableId stuff, and it looks great. I also skimmed over the generator code. On Fri, Oct 1, 2010 at 5:50 PM, cromwell...@google.com wrote: Small nit. Besides validation of the context methods themselves, they also need to be matched up against the service class, I've already done the @ProxyFor matching part in the patch I sent, so I could do the @Service matching if you want. http://gwt-code-reviews.appspot.com/924801/diff/13001/7088 File user/src/com/google/gwt/requestfactory/rebind/model/RequestFactoryModel.java (right): http://gwt-code-reviews.appspot.com/924801/diff/13001/7088#newcode288 user/src/com/google/gwt/requestfactory/rebind/model/RequestFactoryModel.java:288: This validates that the return type is transportable, but does not validate that the parameters are transportable. http://gwt-code-reviews.appspot.com/924801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Use generics for EntityProxyId (issue888802)
http://gwt-code-reviews.appspot.com/935801 On Wed, Sep 29, 2010 at 6:19 PM, Daniel Rice (דניאל רייס) r...@google.comwrote: Somehow a class like P extends BlahBlahBlah was being treated as real class, resulting in a generated PImpl class which broke all kinds of ways. I need to put a breakpoint at that line to see if it still happens and try to deal with the issue further upstream. Dan On Wed, Sep 29, 2010 at 7:58 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/02/diff/1/24 File user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java (right): http://gwt-code-reviews.appspot.com/02/diff/1/24#newcode241 user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java:241: if (P.equals(publicProxyType.getName())) { Uh...what? http://gwt-code-reviews.appspot.com/02/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Removed the acquire event, renamed WriteOperation to DataEvent, renamed CREATE to PERSIST. (issue892802)
Thanks for the feedback. The new patch incorporates the feedback. On Sun, Sep 26, 2010 at 2:41 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/892802/diff/1/2 File fixWriteOperationEnum.diff (right): http://gwt-code-reviews.appspot.com/892802/diff/1/2#newcode1 fixWriteOperationEnum.diff:1: diff --git a/google3/third_party/java_src/gwt/svn/trunk/samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.java b/google3/third_party/java_src/gwt/svn/trunk/samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.java I doubt you meant to check in this diff file. http://gwt-code-reviews.appspot.com/892802/diff/1/4 File user/src/com/google/gwt/app/place/AbstractProxyListActivity.java (right): http://gwt-code-reviews.appspot.com/892802/diff/1/4#newcode195 user/src/com/google/gwt/app/place/AbstractProxyListActivity.java:195: update(event.getDataEvent(), event.getProxyId()); We didn't talk about a rename for this enum, and this is a terrible name. Asking an event for its event? http://gwt-code-reviews.appspot.com/892802/diff/1/5 File user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java (right): http://gwt-code-reviews.appspot.com/892802/diff/1/5#newcode256 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:256: assert master.records.containsKey(proxyId); What changed here? http://gwt-code-reviews.appspot.com/892802/diff/1/9 File user/src/com/google/gwt/requestfactory/client/impl/ValueStoreJsonImpl.java (right): http://gwt-code-reviews.appspot.com/892802/diff/1/9#newcode94 user/src/com/google/gwt/requestfactory/client/impl/ValueStoreJsonImpl.java:94: if (!isFuture) { What's going on here? Your patch description doesn't mention fixing any bugs in what events are posted when, or change the semantics. http://gwt-code-reviews.appspot.com/892802/diff/1/15 File user/test/com/google/gwt/requestfactory/client/RequestFactoryStringTest.java (right): http://gwt-code-reviews.appspot.com/892802/diff/1/15#newcode130 user/test/com/google/gwt/requestfactory/client/RequestFactoryStringTest.java:130: assertEquals(1, handler.updateEventCount); So we do post both a persist event and an update event on create? Not saying that's a bad thing, just want to know and want to see it documented. http://gwt-code-reviews.appspot.com/892802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Cleanup for Roo-1401. Refactored the commit method so that it is easier for it (issue920801)
Thanks. done and commited as http://code.google.com/p/google-web-toolkit/source/detail?r=8868 On Fri, Sep 24, 2010 at 5:09 PM, rj...@google.com wrote: LGTM one name nit http://gwt-code-reviews.appspot.com/920801/diff/3/3001 File user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java (right): http://gwt-code-reviews.appspot.com/920801/diff/3/3001#newcode128 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:128: public void commit(JavaScriptObject returnedJso) { Rename to something more evocative, e.g. processFuturesAndPostEvents (if that's accurate) http://gwt-code-reviews.appspot.com/920801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix a corner case in getProxyId. Minor cleanup. (issue914801)
Incorporates feedback. On Thu, Sep 23, 2010 at 11:27 AM, amitman...@google.com wrote: http://gwt-code-reviews.appspot.com/914801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixed https://jira.springsource.org/browse/ROO-1429 (issue915801)
updated. On Thu, Sep 23, 2010 at 12:52 PM, amitman...@google.com wrote: http://gwt-code-reviews.appspot.com/915801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-introduces the RpcServletUtils.readContentAsUtf8() methods for backwards compatibility (issue887803)
LGTM On Thu, Sep 23, 2010 at 2:50 PM, fre...@google.com wrote: LGTM http://gwt-code-reviews.appspot.com/887803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Some Small logging cleanup (issue875802)
LGTM, with the change that the testServerFailure() in RequestFactoryTest be split into two, as we talked about in person. On Wed, Sep 22, 2010 at 11:48 AM, unn...@google.com wrote: Reviewers: amitmanjhi, Description: Some Small logging cleanup - Use server error reporting in RF - Some documentation fixes - Create inherits module for projects using logging classes but not enabling loggin Please review this at http://gwt-code-reviews.appspot.com/875802/show Affected files: A user/src/com/google/gwt/logging/LoggingDisabled.gwt.xml M user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java M user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml M user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java M user/src/com/google/gwt/requestfactory/server/Logging.java M user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java M user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java M user/test/com/google/gwt/requestfactory/server/SimpleFoo.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: Fixes two Roo issues: ROO-1238 (Amit) and ROO-1380 (pdr) (issue897801)
Thanks Phil and Ray. On Sat, Sep 18, 2010 at 12:12 PM, Ray Ryan rj...@google.com wrote: LGTM++ On Sat, Sep 18, 2010 at 12:11 PM, p...@google.com wrote: http://gwt-code-reviews.appspot.com/897801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] stableId
stableId is not a field in the EntityProxy. It is computed when stableId() is called on an EntityProxy. Does that help? On Mon, Sep 20, 2010 at 10:36 AM, Patrick Julien pjul...@gmail.com wrote: What exactly fills in this field currently? This field is always null for me on the client -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Replaces the public and obnoxious String EntityProxy#getId() with the (issue902801)
checkstyle fails probably because file name=trunk/user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java error line=72 column=5 severity=error message=getEncodedId is not alphabetical. source=com.google.gwt.checkstyle.OrderCheck/ /file file name=trunk/user/src/com/google/gwt/requestfactory/shared/EntityProxyId.java error line=25 severity=error message=Type Javadoc comment is missing an @param lt;Pgt; tag. source=com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTypeCheck/ /file More comments coming... On Mon, Sep 20, 2010 at 8:49 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/902801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Replaces the public and obnoxious String EntityProxy#getId() with the (issue902801)
LGTM, with a TODO (amitmanjhi) to change SimpleFoo id to String and checking if everything works. On Mon, Sep 20, 2010 at 9:15 PM, Amit Manjhi amitman...@google.com wrote: checkstyle fails probably because file name=trunk/user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java error line=72 column=5 severity=error message=getEncodedId is not alphabetical. source=com.google.gwt.checkstyle.OrderCheck/ /file file name=trunk/user/src/com/google/gwt/requestfactory/shared/EntityProxyId.java error line=25 severity=error message=Type Javadoc comment is missing an @param lt;Pgt; tag. source=com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTypeCheck/ /file More comments coming... On Mon, Sep 20, 2010 at 8:49 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/902801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: Remove SyncResults. Use EntityProxyId and RequestFactory.find(EntityProxyId) (issue887802)
The stable id encodes the datastore id and the entity-type. If the RequestFactory find method cannot lookup the datastoreId for a stableId, a client side exception is thrown. On Thu, Sep 16, 2010 at 9:23 AM, Patrick Julien pjul...@gmail.com wrote: On Thu, Sep 16, 2010 at 10:30 AM, Ray Ryan rj...@google.com wrote: For now, via the same static find methods you already provide. As quickly as we can manage it, via the same service object api already discussed. I'm sorry if I am slow but how is the stable id alone sufficient to find whatever it is you're looking for on the server? -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: JUnit ought to just serialize exceptions (issue870802)
[+unnurg] who has graciously offered to review this change. On Wed, Sep 15, 2010 at 10:15 AM, k...@google.com wrote: IIUC, ExceptionWrapper can be removed now after removing the last references in JUnitResult. http://gwt-code-reviews.appspot.com/870802/diff/1/3 File user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java (right): http://gwt-code-reviews.appspot.com/870802/diff/1/3#newcode26 user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java:26: * Stand-in for the non-serializable {...@link Throwable#getCause()}. Isn't Throwable serializable now? http://gwt-code-reviews.appspot.com/870802/diff/1/3#newcode44 user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java:44: private transient boolean exceptionIsInitialized; isInitialized or isExceptionInitialized http://gwt-code-reviews.appspot.com/870802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix warnings and checkstyle errors (issue882802)
LGTM On Wed, Sep 15, 2010 at 11:17 AM, r...@google.com wrote: Reviewers: amitmanjhi, Description: Fix warnings and checkstyle errors Please review this at http://gwt-code-reviews.appspot.com/882802/show Affected files: M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/Expenses.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesMobile.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseDetails.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseEntry.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileReportEntry.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/request/ExpenseProxy.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ui/employee/EmployeeDetailsView.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ui/employee/EmployeeEditView.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ui/report/ReportDetailsView.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ui/report/ReportEditView.java M samples/expenses/src/main/java/com/google/gwt/sample/expenses/server/ReportGenerator.java M user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java M user/src/com/google/gwt/app/place/AbstractProxyListView.java M user/src/com/google/gwt/app/place/WithTokenizers.java M user/src/com/google/gwt/cell/client/IconCellDecorator.java M user/src/com/google/gwt/editor/client/adapters/ValueBoxEditor.java M user/src/com/google/gwt/editor/rebind/model/EditorAccess.java M user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java M user/src/com/google/gwt/requestfactory/client/impl/ProxyToTypeMap.java M user/src/com/google/gwt/requestfactory/shared/RequestEvent.java M user/src/com/google/gwt/requestfactory/shared/RequestObject.java M user/src/com/google/gwt/uibinder/elementparsers/UiChildParser.java M user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java M user/src/com/google/gwt/user/client/ui/HTML.java M user/src/com/google/gwt/user/client/ui/HasOneWidget.java M user/src/com/google/gwt/user/client/ui/IsWidget.java M user/src/com/google/gwt/validation/client/constraints/PastValidatorForDate.java M user/src/com/google/gwt/validation/client/impl/AbstractBeanDescriptor.java M user/src/com/google/gwt/validation/client/impl/ConstraintDescriptorImpl.java M user/src/com/google/gwt/validation/client/impl/GwtSpecificValidator.java M user/src/com/google/gwt/validation/client/impl/NodeImpl.java M user/src/com/google/gwt/validation/rebind/AbstractCreator.java M user/test/com/google/gwt/app/place/ActivityManagerTest.java M user/test/com/google/gwt/app/place/PlaceHistoryHandlerGeneratorTest.java M user/test/com/google/gwt/validation/client/SimpleSample.java M user/test/com/google/gwt/validation/client/constraints/DecimalMaxValidatorForNumberTest.java M user/test/com/google/gwt/validation/client/constraints/DecimalMaxValidatorForStringTest.java M user/test/com/google/gwt/validation/client/constraints/DecimalMinValidatorForNumberTest.java M user/test/com/google/gwt/validation/client/constraints/DecimalMinValidatorForStringTest.java M user/test/com/google/gwt/validation/client/constraints/DigitsValidatorForNumberTest.java M user/test/com/google/gwt/validation/client/constraints/DigitsValidatorForStringTest.java M user/test/com/google/gwt/validation/client/constraints/FutureValidatorForDateTest.java M user/test/com/google/gwt/validation/client/constraints/GwtCompileTest.java M user/test/com/google/gwt/validation/client/constraints/MaxValidatorForNumberTest.java M user/test/com/google/gwt/validation/client/constraints/MaxValidatorForStringTest.java M user/test/com/google/gwt/validation/client/constraints/MinValidatorForNumberTest.java M user/test/com/google/gwt/validation/client/constraints/MinValidatorForStringTest.java M user/test/com/google/gwt/validation/client/constraints/NotNullValidatorTest.java M user/test/com/google/gwt/validation/client/constraints/PastValidatorForDateTest.java M user/test/com/google/gwt/validation/client/constraints/PatternValidatorTest.java M user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfBooleanTest.java M user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfByteTest.java M user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfCharTest.java M user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfDoubleTest.java M user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfFloatTest.java M user/test/com/google/gwt/validation/client/constraints/SizeValidatorForArrayOfIntTest.java M
[gwt-contrib] Re: Remove SyncResults. Use EntityProxyId and RequestFactory.find(EntityProxyId) (issue887802)
That portion has not been fixed. I was planning to get to it after this patch. I prioritized this patch because this patch makes an api change, where as the under-reporting is an implementation bug. Since it is unlikely that I will get to the under-reporting bug today, I will create another bug with a priority (major/critical) to track it. If no one gets to it in my absence, I will finish it next week. On Wed, Sep 15, 2010 at 8:05 PM, rj...@google.com wrote: Amit, https://jira.springsource.org/browse/ROO-1238 includes and fix under-reporting in its title, and my understanding is that this is addressed by your design at https://wave.google.com/wave/waveref/googlewave.com/w+ywx8dL_XC , in particular the portion where you talk about using version comparision. I don't see anything in this patch that touches the server side, and I see no use of version in the servlet. Have the under reporting problems been addressed yet? If they don't require a version comparison after all, what was the fix, in which patch? And why is getVersion still required by EntityProxy. If that issue is still not addressed, either this bug cannot be closed or we need a separate one addressing it. On 2010/09/15 22:12:12, amitmanjhi wrote: http://gwt-code-reviews.appspot.com/887802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Initial implementation of a general purpose Find service based on an (issue878801)
Updated another patch. On Tue, Sep 14, 2010 at 5:21 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/878801/diff/1/6 File user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (right): http://gwt-code-reviews.appspot.com/878801/diff/1/6#newcode296 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:296: Long.class); Shouldn't you make this Object.class to keep from creating more work for Stephany? That fails with Method not found since the method signature still expects Long (in our tests, samples, etc.). http://gwt-code-reviews.appspot.com/878801/diff/1/8 File user/src/com/google/gwt/requestfactory/shared/RequestFactory.java (right): http://gwt-code-reviews.appspot.com/878801/diff/1/8#newcode39 user/src/com/google/gwt/requestfactory/shared/RequestFactory.java:39: FindRequest findRequest(); What happened to making the public api ProxyRequestEntityProxy find(EntityProxyId proxyId) And putting this declaration into RFJsonImpl, along with an impl of find(EntityProxyId) that calls it. We're trying to freeze the API, no one else can swing back and clean it up. Last chance. Fixed in the just uploaded patch. http://gwt-code-reviews.appspot.com/878801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Extract violations from being a field in sideEffects to a top-level field. Removed the handling ... (issue846802)
RequestData.SIDE_EFFECTS_TOKEN is still being used at one place. Commiting the change. On Mon, Sep 13, 2010 at 10:03 AM, rj...@google.com wrote: LGTM http://gwt-code-reviews.appspot.com/846802/diff/1/8 File user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (right): http://gwt-code-reviews.appspot.com/846802/diff/1/8#newcode672 user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java:672: envelop.put(violations, violationsAsJson); If the RequestData.SIDE_EFFECTS_TOKEN constant is no longer used, please delete it. http://gwt-code-reviews.appspot.com/846802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Updates RequestFactory's Receiver type with onViolation() and onFailure() methods, making it an ... (issue855802)
LGTM. On Mon, Sep 13, 2010 at 9:48 PM, Ray Ryan rj...@google.com wrote: LGTM Your last patch looks good, and doesn't break addon-gwt On Mon, Sep 13, 2010 at 6:45 PM, rj...@google.com wrote: Oops, not quite http://gwt-code-reviews.appspot.com/855802/diff/10001/11003 File user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java (left): http://gwt-code-reviews.appspot.com/855802/diff/10001/11003#oldcode127 user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java:127: record = cast(syncRecord); Can't quite get rid of this yet, until Amit finishes the new find method. We need an accurate proxy to make the place change when we're done. http://gwt-code-reviews.appspot.com/855802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] RR: Record (aka Proxy) equality semantics
https://wave.google.com/wave/?pli=1#restored:wave:googlewave.com%252Fw%252BOniKcQcnJ.1 The wave linked above describes how Record equality will work in GWT 2.1. It should be world readable, and editable by everyone in Google-Web-Toolkit-Contributors. Please provide feedback via separate blips, rather than editing the main text directly. Regards, Amit -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Lose idiotic, inflexible password block. (issue779803)
LGTM On Thu, Aug 26, 2010 at 2:31 PM, rj...@google.com wrote: Reviewers: amitmanjhi, Description: Lose idiotic, inflexible password block. Please review this at http://gwt-code-reviews.appspot.com/779803/show Affected files: M user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java Index: user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java === --- user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (revision 8652) +++ user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (working copy) @@ -121,17 +121,7 @@ public static final String RELATED = related; - public static final SetString BLACK_LIST = initBlackList(); - private static final Logger log = Logger.getLogger(JsonRequestProcessor.class.getName()); - - public static SetString initBlackList() { -SetString blackList = new HashSetString(); -for (String str : new String[] {password}) { - blackList.add(str); -} -return Collections.unmodifiableSet(blackList); - } private RequestProperty propertyRefs; @@ -1047,9 +1037,9 @@ RequestProperty propertyContext) { if (Record.class.isAssignableFrom(p.getType())) { return propertyContext.hasProperty(p.getName()); -} else { - return !BLACK_LIST.contains(p.getName()); -} +} + +return true; } /** -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] RR: design doc for ValueStore and Events
https://wave.google.com/wave/#restored:search,restored:wave:googlewave.com%252Fw%252Bywx8dL_XC The wave linked above describes how ValueStore and firing of data events (creates, updates, and deletes) will work in GWT 2.1. It should be world readable, and editable by everyone in Google-Web-Toolkit-Contributors. (I just started a new wave with everyone on the RequestFactory design doc wave.) Please provide feedback via separate blips, rather than editing the main text directly. Regards, Amit -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix for date serialization problem in JsonRequestProcessor. Datanucleus returns custom subtypes ... (issue790801)
LGTM. On Sun, Aug 22, 2010 at 2:07 AM, cromwell...@google.com wrote: Reviewers: amitmanjhi, rjrjr, Description: Fix for date serialization problem in JsonRequestProcessor. Datanucleus returns custom subtypes of java.util.Date from the datastore, which causes Dates to be serialized as english strings. Please review this at http://gwt-code-reviews.appspot.com/790801/show Affected files: M user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java Index: user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java === --- user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (revision 8616) +++ user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java (working copy) @@ -320,7 +320,7 @@ if (Boolean.class == type) { return value; } -if (Date.class == type) { +if (Date.class.isAssignableFrom(type)) { return String.valueOf(((Date) value).getTime()); } if (Enum.class.isAssignableFrom(type)) { -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Restores our new friend ValueListBox, and muzzles a couple of his (issue788801)
LGTM On Fri, Aug 20, 2010 at 1:30 AM, rj...@google.com wrote: Reviewers: amitmanjhi, Description: Restores our new friend ValueListBox, and muzzles a couple of his unit tests which seem to fail on FireFox. The widget works well manually, and M3 needs it. Review by: amitman...@google.com Please review this at http://gwt-code-reviews.appspot.com/788801/show Affected files: M bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Scaffold.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobile.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeDetailsView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditView.ui.xml M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListView.java A bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeRenderer.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportDetailsView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditView.ui.xml M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListView.java M user/src/com/google/gwt/app/client/EditorSupport.java D user/src/com/google/gwt/app/client/ProxyBox.java D user/src/com/google/gwt/app/client/ProxyIdRenderer.java D user/src/com/google/gwt/app/client/ProxyParser.java D user/src/com/google/gwt/app/client/ProxyRenderer.java D user/src/com/google/gwt/app/client/ReadonlyProxyBox.java M user/src/com/google/gwt/app/place/AbstractRecordEditActivity.java M user/src/com/google/gwt/app/place/AbstractRecordListActivity.java M user/src/com/google/gwt/app/place/AbstractRecordListView.java M user/src/com/google/gwt/app/place/PropertyColumn.java M user/src/com/google/gwt/app/place/PropertyView.java A user/src/com/google/gwt/app/place/ProxyRenderer.java M user/src/com/google/gwt/app/rebind/EditorSupportGenerator.java M user/src/com/google/gwt/user/client/ui/HasConstrainedValue.java A user/src/com/google/gwt/user/client/ui/ValueListBox.java M user/src/com/google/gwt/user/client/ui/ValuePicker.java M user/test/com/google/gwt/requestfactory/server/SimpleFoo.java M user/test/com/google/gwt/user/UISuite.java M user/test/com/google/gwt/user/client/ui/ValueBoxBaseTest.java A user/test/com/google/gwt/user/client/ui/ValueListBoxTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: chrome broken
Can you try John's fix at http://gwt-code-reviews.appspot.com/760803/show ? Thanks. On Fri, Aug 20, 2010 at 12:33 PM, Pascal Patry i...@invalidip.com wrote: On Friday, August 20, 2010 15:31:55 John LaBanca wrote: We tracked it down to a Chrome dev mode oddity. Chrome dev mode adds a __gwt_ObjectId attribute to all objects passed between Java and JSNI. I'm testing a simple fix now. Great, thank you -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: fix ExpenseTree for ant -f samples/expenses/build.xml build (issue781802)
LGTM On Fri, Aug 20, 2010 at 5:04 PM, k...@google.com wrote: Reviewers: amitmanjhi, Description: fix ExpenseTree for ant -f samples/expenses/build.xml build Please review this at http://gwt-code-reviews.appspot.com/781802/show Affected files: M samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseTree.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Wrap primitive non-record types in same JSON wrapper used for Records (issue777801)
Quick LGTM. I'll take another detailed look tomorrow. On Tue, Aug 17, 2010 at 11:18 PM, cromwell...@google.com wrote: Reviewers: amitmanjhi, Ray Ryan, Description: Wrap primitive non-record types in same JSON wrapper used for Records Please review this at http://gwt-code-reviews.appspot.com/777801/show Affected files: M user/src/com/google/gwt/requestfactory/client/impl/AbstractBigDecimalRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractBigIntegerRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractBooleanRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractByteRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractCharacterRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractDateRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractDoubleRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractEnumRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractFloatRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractIntegerRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonListRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonObjectRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractLongRequest.java A user/src/com/google/gwt/requestfactory/client/impl/AbstractPrimitiveRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractShortRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractVoidRequest.java M user/src/com/google/gwt/requestfactory/client/impl/RecordJsoImpl.java M user/src/com/google/gwt/requestfactory/client/impl/RequestFactoryJsonImpl.java M user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java M user/src/com/google/gwt/requestfactory/shared/RequestData.java M user/src/com/google/gwt/requestfactory/shared/RequestObject.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Wrap primitive non-record types in same JSON wrapper used for Records (issue777801)
LGTM. Minor nits: - handlePrimitiveResult(...) method can be made protected. Plus, it is missing @Override in all classes but AbstractPrimitiveRequest - the asString(..) method can be moved to AbstractPrimitiveRequest class and made a private member. You would need re-sorting after the above 2 changes. On Wed, Aug 18, 2010 at 9:42 AM, cromwell...@google.com wrote: http://gwt-code-reviews.appspot.com/777801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add @Override tags in order to be able to enable Eclipse warnings without being drowned (issue754801)
Does GWT mandate the eclipse setting that throws warnings for missing @Override? I, for example, had it set to ignore. On Wed, Aug 11, 2010 at 9:23 AM, r...@google.com wrote: Reviewers: amitmanjhi, Description: Add @Override tags in order to be able to enable Eclipse warnings without being drowned Please review this at http://gwt-code-reviews.appspot.com/754801/show Affected files: M bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldDetailsActivities.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeDetailsActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportDetailsActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListActivity.java M bikeshed/src/com/google/gwt/sample/expenses/server/AppCacheWarmer.java M bikeshed/src/com/google/gwt/sample/expenses/server/domain/Employee.java M bikeshed/src/com/google/gwt/sample/expenses/server/domain/Expense.java M bikeshed/src/com/google/gwt/sample/expenses/server/domain/GaeUserInformation.java M bikeshed/src/com/google/gwt/sample/expenses/server/domain/Report.java M dev/core/src/com/google/gwt/core/ext/TreeLogger.java M dev/core/src/com/google/gwt/core/ext/typeinfo/JConstructor.java M dev/core/src/com/google/gwt/core/ext/typeinfo/JField.java M dev/core/src/com/google/gwt/core/ext/typeinfo/JMethod.java M dev/core/src/com/google/gwt/dev/cfg/ConditionAll.java M dev/core/src/com/google/gwt/dev/cfg/ConditionAny.java M dev/core/src/com/google/gwt/dev/cfg/ConditionNone.java M dev/core/src/com/google/gwt/dev/cfg/ConditionWhenPropertyIs.java M dev/core/src/com/google/gwt/dev/cfg/ConditionWhenTypeAssignableTo.java M dev/core/src/com/google/gwt/dev/cfg/ConditionWhenTypeIs.java M dev/core/src/com/google/gwt/dev/cfg/RuleFail.java M dev/core/src/com/google/gwt/dev/cfg/RuleGenerateWith.java M dev/core/src/com/google/gwt/dev/cfg/RuleReplaceWith.java M dev/core/src/com/google/gwt/dev/generator/ast/BaseNode.java M dev/core/src/com/google/gwt/dev/generator/ast/Expression.java M dev/core/src/com/google/gwt/dev/javac/JsniChecker.java M dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java M dev/core/src/com/google/gwt/dev/jdt/FindJsniRefVisitor.java M dev/core/src/com/google/gwt/dev/jjs/ast/JArrayType.java M dev/core/src/com/google/gwt/dev/jjs/ast/JBooleanLiteral.java M dev/core/src/com/google/gwt/dev/jjs/ast/JDoubleLiteral.java M dev/core/src/com/google/gwt/dev/jjs/ast/JFloatLiteral.java M dev/core/src/com/google/gwt/dev/jjs/ast/JModVisitor.java M dev/core/src/com/google/gwt/dev/jjs/ast/JNode.java M dev/core/src/com/google/gwt/dev/jjs/ast/JNullLiteral.java M dev/core/src/com/google/gwt/dev/jjs/ast/JPostfixOperation.java M dev/core/src/com/google/gwt/dev/jjs/ast/JPrefixOperation.java M dev/core/src/com/google/gwt/dev/jjs/ast/JPrimitiveType.java M dev/core/src/com/google/gwt/dev/jjs/ast/JStringLiteral.java M dev/core/src/com/google/gwt/dev/jjs/ast/JUnaryOperator.java M dev/core/src/com/google/gwt/dev/jjs/ast/js/JsniMethodRef.java M dev/core/src/com/google/gwt/dev/jjs/impl/AssertionNormalizer.java M dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java M dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java M dev/core/src/com/google/gwt/dev/jjs/impl/JsIEBlockTextTransformer.java M dev/core/src/com/google/gwt/dev/js/JsConstructExpressionVisitor.java M dev/core/src/com/google/gwt/dev/js/JsFirstExpressionVisitor.java M dev/core/src/com/google/gwt/dev/js/JsHoister.java M dev/core/src/com/google/gwt/dev/js/JsRequiresSemiVisitor.java M dev/core/src/com/google/gwt/dev/js/ast/JsArrayLiteral.java M dev/core/src/com/google/gwt/dev/js/ast/JsBooleanLiteral.java M dev/core/src/com/google/gwt/dev/js/ast/JsBreak.java M dev/core/src/com/google/gwt/dev/js/ast/JsConditional.java M dev/core/src/com/google/gwt/dev/js/ast/JsContinue.java M dev/core/src/com/google/gwt/dev/js/ast/JsFunction.java M dev/core/src/com/google/gwt/dev/js/ast/JsModVisitor.java M dev/core/src/com/google/gwt/dev/js/ast/JsNullLiteral.java M dev/core/src/com/google/gwt/dev/js/ast/JsNumberLiteral.java M dev/core/src/com/google/gwt/dev/js/ast/JsObjectLiteral.java M dev/core/src/com/google/gwt/dev/js/ast/JsRegExp.java M dev/core/src/com/google/gwt/dev/js/ast/JsReturn.java M dev/core/src/com/google/gwt/dev/js/ast/JsStringLiteral.java M dev/core/src/com/google/gwt/dev/js/ast/JsThisRef.java M dev/core/src/com/google/gwt/dev/js/ast/JsThrow.java M dev/core/src/com/google/gwt/dev/shell/EmmaStrategy.java M dev/core/src/com/google/gwt/dev/shell/GWTBridgeImpl.java M
Re: [gwt-contrib] Re: Add @Override tags in order to be able to enable Eclipse warnings without being drowned (issue754801)
LGTM We anyway recommend the 1.5 compatibility setting. On Wed, Aug 11, 2010 at 10:49 AM, Daniel Rice (דניאל רייס) r...@google.comwrote: I've had it off but I noticed in one of the changes this morning that an @Override tag was (correctly) added and was curious enough to check the setting. I would like to be able to enable warnings in my own Eclipse since there are benefits to getting it right (namely avoiding accidental non-overrides due to misspelling). This CL would make it reasonable for those who want to to enable the warning. Dan On Wed, Aug 11, 2010 at 1:22 PM, John Tamplin j...@google.com wrote: On Wed, Aug 11, 2010 at 1:14 PM, Amit Manjhi amitman...@google.comwrote: Does GWT mandate the eclipse setting that throws warnings for missing @Override? I, for example, had it set to ignore. It is recommended and useful. The one catch is that if you don't tell Eclipse to use JDK1.5 compatibility, it will insist on @Overrides on methods implementing an interface, which is not allowed in 1.5. -- John A. Tamplin Software Engineer (GWT), Google -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Another step towards making instance methods real. The interface on the client is pretty much wh... (issue710803)
As we chatted over IM, the RequestDataManager class was not deleted in an earlier patch, as it should have been. Prepared a patch that deletes the class: http://gwt-code-reviews.appspot.com/743801/show On Fri, Aug 6, 2010 at 10:14 AM, cromwell...@google.com wrote: http://gwt-code-reviews.appspot.com/710803/diff/1/19 File user/src/com/google/gwt/requestfactory/shared/impl/RequestDataManager.java (right): http://gwt-code-reviews.appspot.com/710803/diff/1/19#newcode63 user/src/com/google/gwt/requestfactory/shared/impl/RequestDataManager.java:63: } Why was this class reinstated? This encodeParameterValue() method isn't being called is it? It is not correct. http://gwt-code-reviews.appspot.com/710803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Treat undefined Record properties on JSOs as nulls (issue744801)
LGTM On Fri, Aug 6, 2010 at 3:58 PM, cromwell...@google.com wrote: Reviewers: amitmanjhi, Description: Treat undefined Record properties on JSOs as nulls Please review this at http://gwt-code-reviews.appspot.com/744801/show Affected files: M user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java M user/test/com/google/gwt/valuestore/shared/impl/RecordJsoImplTest.java Index: user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java === --- user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java (revision 8500) +++ user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java (working copy) @@ -83,12 +83,12 @@ // assert isDefined(property.getName()) : // Cannot ask for a property before setting it: // + property.getName(); -if (isNull(property.getName())) { +if (isNullOrUndefined(property.getName())) { return null; } try { if (Boolean.class.equals(property.getType())) { - return (V) Boolean.valueOf((String) get(property.getName())); + return (V) Boolean.valueOf(getBoolean(property.getName())); } if (Byte.class.equals(property.getType())) { return (V) Byte.valueOf((byte) getInt(property.getName())); @@ -189,8 +189,8 @@ /** * @param name */ - public final native boolean isNull(String name)/*-{ -return this[name] === null; + public final native boolean isNullOrUndefined(String name)/*-{ +return this[name] == null; }-*/; public final boolean merge(RecordJsoImpl from) { @@ -299,6 +299,10 @@ return @java.util.Date::createFrom(D)(millis); }-*/; + private native boolean getBoolean(String name) /*-{ +return this[name]; + }-*/; + private native double getDouble(String name) /*-{ return this[name]; }-*/; Index: user/test/com/google/gwt/valuestore/shared/impl/RecordJsoImplTest.java === --- user/test/com/google/gwt/valuestore/shared/impl/RecordJsoImplTest.java (revision 8500) +++ user/test/com/google/gwt/valuestore/shared/impl/RecordJsoImplTest.java (working copy) @@ -122,6 +122,8 @@ } assertEquals((Long) 42L, jso.getId()); assertEquals(new Integer(1), jso.getVersion()); +assertEquals(null, jso.get(SimpleFooRecord.longField)); +assertEquals(null, jso.get(SimpleFooRecord.enumField)); testSchema(jso, schemaPresent); } -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Moved the create method from DeltaValueStore, used Long futureIds. Deleted ValueStore interface ... (issue721805)
checked. It does not do any creates and so is fine. On Thu, Aug 5, 2010 at 12:31 PM, rj...@google.com wrote: LGTM provided DynaTableRF sample is not broken http://gwt-code-reviews.appspot.com/721805/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix support for null fields and Boolean type types in RequestFactory. (issue739801)
imo, the client should independently check for errors, without relying on the server code. The server code could be stale, for example. How about if the previous code, i.e., the code before the patch, is rearranged to check for null return value from getInt(...)? On Wed, Aug 4, 2010 at 12:35 AM, cromwell...@google.com wrote: http://gwt-code-reviews.appspot.com/739801/diff/3001/1013 File user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java (right): http://gwt-code-reviews.appspot.com/739801/diff/3001/1013#newcode131 user/src/com/google/gwt/valuestore/shared/impl/RecordJsoImpl.java:131: return null; Yes, but in theory this shouldn't happen since the domain object on the server is returning Long, and we should be aggressively checking that they match up at invocation time. If the domain object is returning String, but the Record is Long, we should be trapping this earlier. On 2010/08/04 07:16:38, amitmanjhi wrote: In addition to gracefully handling a Long field being set to null, wouldn't this return null even if a Long field is set to a String foo? http://gwt-code-reviews.appspot.com/739801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Forgot to commit .classpath and .project for DynaTableRf due to (issue721802)
LGTM On Thu, Jul 29, 2010 at 12:47 PM, rj...@google.com wrote: Reviewers: amitmanjhi, Description: Forgot to commit .classpath and .project for DynaTableRf due to .gitignore settings. Review by: amitman...@google.com Please review this at http://gwt-code-reviews.appspot.com/721802/show Affected files: A eclipse/samples/DynaTableRf/.classpath A eclipse/samples/DynaTableRf/.project Index: eclipse/samples/DynaTableRf/.classpath === --- eclipse/samples/DynaTableRf/.classpath (revision 0) +++ eclipse/samples/DynaTableRf/.classpath (revision 0) @@ -0,0 +1,8 @@ +?xml version=1.0 encoding=UTF-8? +classpath + classpathentry kind=src path=core/src/ + classpathentry kind=src output=war path=core/war/ + classpathentry kind=con path=org.eclipse.jdt.launching.JRE_CONTAINER/ + classpathentry combineaccessrules=false kind=src path=/gwt-user/ + classpathentry kind=output path=war/WEB-INF/classes/ +/classpath Index: eclipse/samples/DynaTableRf/.project === --- eclipse/samples/DynaTableRf/.project(revision 0) +++ eclipse/samples/DynaTableRf/.project(revision 0) @@ -0,0 +1,30 @@ +?xml version=1.0 encoding=UTF-8? +projectDescription + nameDynaTableRf/name + comment/comment + projects + /projects + buildSpec + buildCommand + nameorg.eclipse.jdt.core.javabuilder/name + arguments + /arguments + /buildCommand + buildCommand + namecom.atlassw.tools.eclipse.checkstyle.CheckstyleBuilder/name + arguments + /arguments + /buildCommand + /buildSpec + natures + natureorg.eclipse.jdt.core.javanature/nature + naturecom.atlassw.tools.eclipse.checkstyle.CheckstyleNature/nature + /natures + linkedResources + link + namecore/name + type2/type + locationURIGWT_ROOT/samples/dynatablerf/locationURI + /link + /linkedResources +/projectDescription -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Drop the bikeshed jar from the GWT distribution. (issue665801)
Yes, it is no longer needed. We can drop it from the maven report for the next milestone. On Jun 25, 2010 6:37 AM, jasonpar...@google.com wrote: Does this mean gwt-bikeshed.jar is no longer needed? Do we need it at all in the Maven repo for the next milestone? http://gwt-code-reviews.appspot.com/665801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Drop the bikeshed jar from the GWT distribution. (issue665801)
There is still a dist-bikeshed target that will produce the bikeshed jar. I did not touch it. Does that cover the use case you have in mind? This CL just ensures that the dist target not produce the bikeshed jar or include the bikeshed jar in the distribution. On Fri, Jun 25, 2010 at 8:01 AM, fabb...@google.com wrote: I have mixed feelings... if bikeshed is null now, do we think we will/won't need such a workspace in the future? I thought something like that was being considered as the incubator replacement model, for example. I suppose we can turn it back on at will, but it seems to me that having build.xml always build the perhaps-empty bikeshed. I'm less sure about distro-source, though... it's explicitly for in-progress work, so I'm not sure it's part of a release even if not empty! On 2010/06/25 14:23:11, amitmanjhi wrote: Yes, it is no longer needed. We can drop it from the maven report for the next milestone. On Jun 25, 2010 6:37 AM, mailto:jasonpar...@google.com wrote: Does this mean gwt-bikeshed.jar is no longer needed? Do we need it at all in the Maven repo for the next milestone? http://gwt-code-reviews.appspot.com/665801/show http://gwt-code-reviews.appspot.com/665801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Wholesale move of the app, requestfactory, and valuestore packages from the bikeshed dir to user... (issue660802)
Thanks for the feedback. Since I already submitted the patch, I will create follow-up patches for the issues below. 1. The javax validation is just used in the RequestFactory servlet. It is currently not used by anything except the expenses sample. So I could get away with not including it, and still not causing immediate damage. That said, we do want to include the dependency in the distribution. However, gwt-dev seems like the wrong place. It would be nice to include it in gwt-servlet jar instead. What do you think? 2. I agree there is nothing bikeshed-y about the maven_script.sh. However, I did not want to clutter of a top-level dir which just has a single script. Move it to tools/scripts? On Fri, Jun 25, 2010 at 8:03 AM, fabb...@google.com wrote: Sorry to get to this a bit late... I've got a couple of concerns: 1. regarding especially the user.compile use of javax validation, does that suggest we should embed that library in gwt-dev.jar with all of our other runtime dependencies? (I don't love that practice, but it has been our practice...) 2. bikeshed/scripts/maven_script.sh seems kind of orphaned if the rest goes away. There's nothing really bikeshed-y about it, is there? Should it move up to a top-level scripts directory? On 2010/06/24 20:29:26, amitmanjhi wrote: http://gwt-code-reviews.appspot.com/660802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Drop the bikeshed jar from the GWT distribution. (issue665801)
Yes, the eclipse plugin code should be modified appropriately and the jar does not need to published by maven. That is why I put you and Jason on the reviewers list :). Can you guys take care of these issues? Thanks! On Fri, Jun 25, 2010 at 10:58 AM, rda...@google.com wrote: LGTM - I'm guessing that the appropriate classes have now been merged into gwt-user proper? Side note: The plugin code can now be modified so that it doesn't have special handling for the bikeshed jar (i.e. putting it at the top of the container). http://gwt-code-reviews.appspot.com/665801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Moved the maven_script.sh from bikeshed/scripts to tools/scripts (issue666801)
Thanks, submitted with an additional change. The script builds 'dist-dev' instead of the now unnecessary 'dist-bikeshed' target. Verified that it works. On Fri, Jun 25, 2010 at 11:35 AM, Freeland Abbott fabb...@google.comwrote: LGTM! On Fri, Jun 25, 2010 at 2:31 PM, amitman...@google.com wrote: Reviewers: fabbott, Description: Moved the maven_script.sh from bikeshed/scripts to tools/scripts Patch by: amitmanjhi Review by: fabbott (TBR) Please review this at http://gwt-code-reviews.appspot.com/666801/show Affected files: D bikeshed/scripts/maven_script.sh A tools/scripts/maven_script.sh Index: bikeshed/scripts/maven_script.sh === --- bikeshed/scripts/maven_script.sh(revision 8316) +++ bikeshed/scripts/maven_script.sh(working copy) @@ -1,29 +0,0 @@ -#!/bin/bash -# -# Copyright 2010 Google Inc. -# -# Licensed under the Apache License, Version 2.0 (the License); you may not -# use this file except in compliance with the License. You may obtain a copy of -# the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an AS IS BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations under -# the License. - - -# This script should be run from the trunk dir. -ant clean dist-bikeshed - -MAVEN_REPO=~/.m2/repository - -for i in dev user servlet -do - mvn install:install-file -DgroupId=com.google.gwt -DartifactId=gwt-${i} -Dversion=2.1.0.M1 -Dpackaging=jar -Dfile=build/lib/gwt-${i}.jar -DgeneratePom=true -done -touch /tmp/empty-fake-soyc-vis.jar -mvn install:install-file -DgroupId=com.google.gwt -DartifactId=gwt-soyc-vis -Dversion=2.1.0.M1 -Dpackaging=jar -DgeneratePom=true -Dfile=/tmp/empty-fake-soyc-vis.jar -echo installed the gwt libs in the maven repo Index: tools/scripts/maven_script.sh === --- tools/scripts/maven_script.sh (revision 0) +++ tools/scripts/maven_script.sh (revision 0) @@ -0,0 +1,29 @@ +#!/bin/bash +# +# Copyright 2010 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the License); you may not +# use this file except in compliance with the License. You may obtain a copy of +# the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an AS IS BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations under +# the License. + + +# This script should be run from the trunk dir. +ant clean dist-bikeshed + +MAVEN_REPO=~/.m2/repository + +for i in dev user servlet +do + mvn install:install-file -DgroupId=com.google.gwt -DartifactId=gwt-${i} -Dversion=2.1.0.M1 -Dpackaging=jar -Dfile=build/lib/gwt-${i}.jar -DgeneratePom=true +done +touch /tmp/empty-fake-soyc-vis.jar +mvn install:install-file -DgroupId=com.google.gwt -DartifactId=gwt-soyc-vis -Dversion=2.1.0.M1 -Dpackaging=jar -DgeneratePom=true -Dfile=/tmp/empty-fake-soyc-vis.jar +echo installed the gwt libs in the maven repo -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Wholesale move of the app, requestfactory, and valuestore packages from the bikeshed dir to user... (issue660802)
Okay, I created a Jira issue (https://jira.springsource.org/browse/ROO-1032) for figuring out how to ship the javax.validation jars in the M3 timeframe. Commited the remaining two fixes you pointed out. On Fri, Jun 25, 2010 at 10:43 AM, Freeland Abbott fabb...@google.comwrote: On Fri, Jun 25, 2010 at 1:23 PM, Amit Manjhi amitman...@google.comwrote: Thanks for the feedback. Since I already submitted the patch, I will create follow-up patches for the issues below. 1. The javax validation is just used in the RequestFactory servlet. It is currently not used by anything except the expenses sample. So I could get away with not including it, and still not causing immediate damage. That said, we do want to include the dependency in the distribution. However, gwt-dev seems like the wrong place. It would be nice to include it in gwt-servlet jar instead. What do you think? I would be squirmy about being version-married in gwt-servlet.jar; what if user's server code also used validation? (We get away with it in gwt-dev, at least so far, but it's almost surprising to me that we do.) I'm not sure how we want to address that. Also, I just mailed you privately, but I think SampleDataPopulator is also broken---it pulls classes off gwt-dev.jar (dev.util.Util), which shouldn't be on the server classpath. 2. I agree there is nothing bikeshed-y about the maven_script.sh. However, I did not want to clutter of a top-level dir which just has a single script. Move it to tools/scripts? Works for me. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduces com.google.gwt.text from bikeshed, along with changes in (issue649801)
LGTM On Fri, Jun 18, 2010 at 3:11 PM, rj...@google.com wrote: Reviewers: amitmanjhi, jgw, Description: Introduces com.google.gwt.text from bikeshed, along with changes in com.google.gwt.user.client.ui to take advantage of it. This is another step toward getting rid of the bikeshed. Note in particular the new TextBox-like widget ValueBox, which required refactoring a new abstract superclass ValueBoxBase out of TextBoxBase. A smaller change to note is the introduction of TakesValue as a slimmer super-interface for HasValue which doesn't require being an event source. The Editor framework is expected to take advantage of this, although that may still change a bit. Items that are still half-baked (IsWidget, BooleanParser, BooleanValueBox, etc.) have been moved to com.google.gwt.app rather than text or user. This required moving some classes (e.g. the various Record*View interfaces) from valuestore to app, to avoid cirular dependencies. Please review this at http://gwt-code-reviews.appspot.com/649801/show Affected files: A bikeshed/src/com/google/gwt/app/client/BooleanParser.java A bikeshed/src/com/google/gwt/app/client/BooleanRenderer.java M bikeshed/src/com/google/gwt/app/client/CellListPlacePickerView.java A bikeshed/src/com/google/gwt/app/client/DoubleBox.java A bikeshed/src/com/google/gwt/app/client/DoubleParser.java A bikeshed/src/com/google/gwt/app/client/DoubleRenderer.java M bikeshed/src/com/google/gwt/app/client/EditorSupport.java A bikeshed/src/com/google/gwt/app/client/IntegerBox.java A bikeshed/src/com/google/gwt/app/client/IntegerParser.java A bikeshed/src/com/google/gwt/app/client/IntegerRenderer.java A bikeshed/src/com/google/gwt/app/client/LongBox.java A bikeshed/src/com/google/gwt/app/client/LongParser.java A bikeshed/src/com/google/gwt/app/client/LongRenderer.java M bikeshed/src/com/google/gwt/app/place/AbstractRecordEditActivity.java M bikeshed/src/com/google/gwt/app/place/AbstractRecordListActivity.java A bikeshed/src/com/google/gwt/app/place/AbstractRecordListView.java M bikeshed/src/com/google/gwt/app/place/Activity.java M bikeshed/src/com/google/gwt/app/place/ActivityManager.java A bikeshed/src/com/google/gwt/app/place/IsWidget.java M bikeshed/src/com/google/gwt/app/place/PlacePicker.java M bikeshed/src/com/google/gwt/app/place/PlacePickerView.java A bikeshed/src/com/google/gwt/app/place/PropertyColumn.java A bikeshed/src/com/google/gwt/app/place/PropertyView.java A bikeshed/src/com/google/gwt/app/place/RecordDetailsView.java A bikeshed/src/com/google/gwt/app/place/RecordEditView.java A bikeshed/src/com/google/gwt/app/place/RecordListView.java D bikeshed/src/com/google/gwt/input/Input.gwt.xml D bikeshed/src/com/google/gwt/input/shared/BooleanParser.java D bikeshed/src/com/google/gwt/input/shared/BooleanRenderer.java D bikeshed/src/com/google/gwt/input/shared/DateTimeFormatRenderer.java D bikeshed/src/com/google/gwt/input/shared/DoubleParser.java D bikeshed/src/com/google/gwt/input/shared/DoubleRenderer.java D bikeshed/src/com/google/gwt/input/shared/IntegerParser.java D bikeshed/src/com/google/gwt/input/shared/IntegerRenderer.java D bikeshed/src/com/google/gwt/input/shared/LongParser.java D bikeshed/src/com/google/gwt/input/shared/LongRenderer.java D bikeshed/src/com/google/gwt/input/shared/ParseException.java D bikeshed/src/com/google/gwt/input/shared/Parser.java D bikeshed/src/com/google/gwt/input/shared/PassthroughParser.java D bikeshed/src/com/google/gwt/input/shared/PassthroughRenderer.java D bikeshed/src/com/google/gwt/input/shared/Renderer.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Scaffold.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobile.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/ScaffoldListPlaceRenderer.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeDetailsActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeDetailsView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeEditView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportDetailsActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportDetailsView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportEditView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListActivity.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListView.java D bikeshed/src/com/google/gwt/user/User.gwt.xml D
Re: [gwt-contrib] Re: Produces the gwt-bikeshed.jar from the dist target. In GPE, the GWT (issue541801)
LGTM On Mon, May 17, 2010 at 5:06 PM, rda...@google.com wrote: LGTM. http://gwt-code-reviews.appspot.com/541801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Stop trying to be clever on delete, it breaks things (issue522801)
LGTM On Thu, May 13, 2010 at 9:53 AM, rj...@google.com wrote: Reviewers: amitmanjhi, Description: Stop trying to be clever on delete, it breaks things Review by: amitman...@google.com Please review this at http://gwt-code-reviews.appspot.com/522801/show Affected files: M /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListActivity.java Index: /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListActivity.java === --- /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListActivity.java (revision 8109) +++ /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListActivity.java (working copy) @@ -36,9 +36,9 @@ * Subclasses must: * * ul - * liimplement a method for creating request objects + * liimplement methods to provide a full count, and request a specific * liprovide a {...@link RecordListView} - * lirespond to show and edit requests + * lirespond to show details commands * /ul * * Only the properties required by the view will be requested. @@ -147,7 +147,7 @@ break; case DELETE: -delete(record); +init(); break; case CREATE: @@ -169,13 +169,6 @@ protected abstract void fireCountRequest(ReceiverLong callback); protected abstract void showDetails(R record); - - private void delete(R record) { -Integer row = recordToRow.get(record.getId()); -if (row != null) { - onRangeChanged(view.asPagingListView()); -} - } private void getLastPage() { fireCountRequest(new ReceiverLong() { -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Allow user friendly names on Property (issue509801)
LGTM On Tue, May 11, 2010 at 10:04 PM, rj...@google.com wrote: Reviewers: amitmanjhi, Description: Allow user friendly names on Property Review by: amitman...@google.com Please review this at http://gwt-code-reviews.appspot.com/509801/show Affected files: M /bikeshed/src/com/google/gwt/sample/expenses/gwt/request/EmployeeRecord.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/request/ExpenseRecord.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/request/ReportRecord.java M /bikeshed/src/com/google/gwt/valuestore/shared/Property.java M /bikeshed/src/com/google/gwt/valuestore/ui/AbstractRecordListView.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Implement Delete, and make forms prettier (issue491801)
Lgtm On May 6, 2010 7:32 PM, rj...@google.com wrote: Verbal LGTM from Amit during desk review, submitting. http://gwt-code-reviews.appspot.com/491801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-enable user ant test for htmlunit (issue475801)
LGTM. HtmlUnit should be used; it should be uncommented everywhere. I uncommented a few things in r7787 (link below), but it seems like I missed a few instances. http://code.google.com/p/google-web-toolkit/source/detail?r=7787path=/trunk/user/build.xml One nit : the assertion that ant -f user/build.xml test doesn't do anything seems incorrect. It still runs HtmlUnit tests. No? This patch fixes ant test.dev, ant test.web, and other similar targets. On Wed, May 5, 2010 at 5:52 PM, sco...@google.com wrote: Reviewers: amitmanjhi, jlabanca, Description: Sorry, for some reason side-by-side diff isn't working for me. This patch is to alleviate the fact that: ant -f user/build.xml test Does practically nothing. Even if hmtlunit is somewhat flaky, it's gotten be better than nothing. Please review this at http://gwt-code-reviews.appspot.com/475801/show Affected files: user/build.xml Index: user/build.xml diff --git a/user/build.xml b/user/build.xml index cac355b5497b7266b714f9ef724bb8476e448b8c..8ffde00190d4b22537009bffbee8ac2f968e9633 100755 --- a/user/build.xml +++ b/user/build.xml @@ -52,6 +52,7 @@ pathelement location=${gwt.tools.lib}/easymock/easymock.jar/ pathelement location=${gwt.tools.lib}/easymock/easymockclassextension.jar/ pathelement location=${gwt.tools.lib}/objectweb/asm-3.1.jar/ +pathelement location=${gwt.dev.jar} / /path !-- Platform shouldn't matter here, just picking one -- @@ -97,15 +98,9 @@ destdir=${javac.junit.out} classpath pathelement location=${javac.out} / -pathelement location=${gwt.build}/out/dev/bin-test / -pathelement location=${gwt.tools.lib}/tomcat/servlet-api-2.5.jar / pathelement location=${gwt.tools.lib}/junit/junit-3.8.1.jar / pathelement location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar / - pathelement location=${gwt.tools.lib}/cglib/cglib-2.2.jar/ - pathelement location=${gwt.tools.lib}/easymock/easymock.jar/ - pathelement location=${gwt.tools.lib}/easymock/easymockclassextension.jar/ - pathelement location=${gwt.tools.lib}/objectweb/asm-3.1.jar/ -pathelement location=${gwt.dev.jar} / +path refid=test.extraclasspath / /classpath /gwt.javac /target @@ -506,9 +501,7 @@ limit failonerror=true hours=${test.timeout} parallel threadsPerProcessor=${gwt.threadsPerProcessor} threadCount=${gwt.threadCount} - !-- disable HtmlUnit until it is reliable antcall target=test.dev.htmlunit/ - -- !-- no-op unless gwt.hosts.dev.remote is defined -- antcall target=test.dev.remote/ !-- no-op unless gwt.hosts.dev.selenium is defined -- @@ -527,9 +520,7 @@ limit failonerror=true hours=${test.timeout} parallel threadsPerProcessor=${gwt.threadsPerProcessor} threadCount=${gwt.threadCount} - !-- disable HtmlUnit until it is reliable antcall target=test.web.htmlunit/ - -- !-- no-op unless gwt.hosts.web.remote is defined -- antcall target=test.web.remote/ !-- no-op unless gwt.hosts.web.selenium is defined -- @@ -548,9 +539,7 @@ limit failonerror=true hours=${test.timeout} parallel threadsPerProcessor=${gwt.threadsPerProcessor} threadCount=${gwt.threadCount} - !-- disable HtmlUnit until it is reliable antcall target=test.emma.htmlunit/ - -- !-- no-op unless gwt.hosts.dev.remote is defined -- antcall target=test.emma.remote/ !-- no-op unless gwt.hosts.dev.selenium is defined -- @@ -569,9 +558,7 @@ limit failonerror=true hours=${test.timeout} parallel threadsPerProcessor=${gwt.threadsPerProcessor} threadCount=${gwt.threadCount} - !-- disable HtmlUnit until it is reliable antcall target=test.draft.htmlunit/ - -- !-- no-op unless gwt.hosts.web.remote is defined -- antcall target=test.draft.remote/ !-- no-op unless gwt.hosts.web.selenium is defined -- @@ -590,9 +577,7 @@ limit failonerror=true hours=${test.timeout} parallel threadsPerProcessor=${gwt.threadsPerProcessor} threadCount=${gwt.threadCount} - !-- disable HtmlUnit until it is reliable antcall target=test.web.htmlunit/ - -- !-- no-op unless gwt.hosts.web.remote is defined -- antcall target=test.nometa.remote/ !-- no-op unless gwt.hosts.web.selenium is defined -- -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Simplifies ActivityManager a bit, and adds a JRE unit test -- not (issue389802)
Sure, I can take a look at it tomorrow morning. On Tue, Apr 27, 2010 at 9:11 PM, rj...@google.com wrote: Ray, any chance of you getting to this tonight? If not, Amit, maybe you can take it in the morning? http://gwt-code-reviews.appspot.com/389802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduces Activity and AcitivityManager. (issue398801)
Change looks great. Btw, I took your change, fixed a few minor things and added ui.xml files for Employee/Report details and also added the edit functionality for Report objects. I have attached the patch relative to the original patch. On Fri, Apr 23, 2010 at 12:05 PM, rj...@google.com wrote: http://gwt-code-reviews.appspot.com/398801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors additionalPatch.diff Description: Binary data
[gwt-contrib] Re: Dramatically simplifies public API of RequestFactory and (issue344804)
LGTM On Fri, Apr 16, 2010 at 3:52 PM, rj...@google.com wrote: Submitted after desk review w/amitmanjhi, who I'm sure will chime in with LGTM soon now. http://gwt-code-reviews.appspot.com/344804/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Multiple inheritance in Widget hierarchy
+1 to both Ray and Scott's answers. On Thu, Mar 18, 2010 at 11:59 AM, Ray Ryan rj...@google.com wrote: It doesn't come up with HTMLUnit because from the GWT point of view HTMLUnit is just another browser. Prod mode is a non-issue, and for dev mode we have the moral equivalent of a browser plugin for it. On Thu, Mar 18, 2010 at 11:45 AM, Joel Webber j...@google.com wrote: John, The background behind that weird type hierarchy is that it stems from a time before overlay types existed. Originally c.g.g.user.client.Element was a simple opaque handle that had to be passed to the DOM.*() methods to get anything done (same for Event). After the compiler got overlay type support we added c.g.g.dom.client.Element as a superclass to user.Element, and refactored all the DOM.*() methods to use the new dom.* node/element hierarchy. Unfortunately we couldn't banish user.Element altogether without breaking everyone, so we ended up with a number of warts like setElement(user.Element). It's been on our TODO list for some time to deprecate user.Element and remove all references to it, but no one's found the time yet. As you've probably discovered, we just end up using JSO.cast() to work around this where necessary, but of course that won't work in real Java. If I understand what you're doing correctly, it sounds like clearing this up would simplify your life. But it might take a while, because we have to go through a fair amount of work to get there. @kathrin amit: How did you guys deal with this in the HtmlUnit testing stuff? Sounds like it would have come up there. Cheers, joel. On Thu, Mar 18, 2010 at 12:45 AM, jd jdpatter...@gmail.com wrote: Hi, I am reposting this question here as the user list got no reply and I guess it is more a dev question: I am experimenting with compiling GWT code with a standard JDK so I can use the same code to generate HTML on both the client and the server. So far it seem to be working OK but will only be practical if I can also get UIBinder and i18n working. My goal is to create HTML pages that can be crawled and indexed and also allow GWT code to add, load and modify the page. Others have recommended building two parallel sites - an html one and a GT one which seems a bit redundant. My experiement has put a real w3c Node inside every GWT Node and replace native methods with ones that manipulate the w3c node. Then finally I take the full w3c node from any element and convert it into html. I found that the object hierarchy needed to be changed to be valid Java. An example of the issue is with the Anchor widget: public Anchor() { setElement(Document.get().createAnchorElement()); setStyleName(gwt-Anchor); } com.google.gwt.dom.client.AnchorElement extends com.google.gwt.dom.client.AnchorElement but setElement expects a com.google.gwt.user.client.Element so AnchorElement must extend both classes which is impossible. I have modified AnchorElement and friends to extends com.google.gwt.user.client.Element instead which seems to have worked. My question is: Is this impossible inheritance hierarchy intentional to stop this kind of messing about? Cheers, John -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduces expenses scaffolding app, ExpensesScaffold, complete with
Followup question. - Is Expenses.gwt.xml necessary with the introduction of ExpensesScaffolding.xml? Can the Expenses.java functionality be moved into ExpensesScaffolding.java? On Wed, Mar 17, 2010 at 11:14 AM, amitman...@google.com wrote: Additional checkstyle problems (Eclipse output). Please run checkstyle :) Description ResourcePathLocationType forProperties is not alphabetical. EntityListRequest.java Bikeshed/src/com/google/gwt/requestfactory/shared line 35 Checkstyle Problem getDetailsPlaceFor is not alphabetical. Places.java Bikeshed/src/com/google/gwt/sample/expenses/client/placeline 34 Checkstyle Problem Line does not match expected header line of '/\*'. ExpensesScaffold.java Bikeshed/src/com/google/gwt/sample/expenses/client line 1 Checkstyle Problem Line does not match expected header line of '/\*'. ListRequester.java Bikeshed/src/com/google/gwt/sample/expenses/client line 1 Checkstyle Problem Line matches the illegal pattern 'newline before }'. ListRequester.java Bikeshed/src/com/google/gwt/sample/expenses/client line 45 Checkstyle Problem http://gwt-code-reviews.appspot.com/209802/diff/1/16 File bikeshed/src/com/google/gwt/sample/expenses/client/ExpensesScaffoldShell.java (right): http://gwt-code-reviews.appspot.com/209802/diff/1/16#newcode32 Line 32: interface Binder extends UiBinderWidget, ExpensesScaffoldShell {} checkstyle. { is not followed by whitespace. http://gwt-code-reviews.appspot.com/209802/diff/1/24 File bikeshed/src/com/google/gwt/sample/expenses/client/place/EntityPlace.java (right): http://gwt-code-reviews.appspot.com/209802/diff/1/24#newcode45 Line 45: if (this == obj) checkstyle: not alphabetical http://gwt-code-reviews.appspot.com/209802/diff/1/34 File bikeshed/src/com/google/gwt/sample/expenses/server/domain/Storage.java (right): http://gwt-code-reviews.appspot.com/209802/diff/1/34#newcode145 Line 145: synchronized ListReport findAllReports() { checkstyle: findAllReports is not alphabetical http://gwt-code-reviews.appspot.com/209802/diff/1/42 File bikeshed/src/com/google/gwt/user/client/ui/ValueListBox.java (right): http://gwt-code-reviews.appspot.com/209802/diff/1/42#newcode87 Line 87: public HandlerRegistration addValueChangeHandler(ValueChangeHandlerT handler) { checkstyle: method is not alphabetical. http://gwt-code-reviews.appspot.com/209802 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: allow skipping unit tests in development or production mode
yeah the implementation looks fine On Mar 11, 2010 8:32 AM, Lex Spoon sp...@google.com wrote: On Wed, Mar 10, 2010 at 7:17 PM, amitman...@google.com wrote: LGTM on this quick and dirty sol... I feel the same way. A quick and dirty solution would be very valuable so that I can add tests for the cross-site linker. However, it looks like we ultimately want to support an or of a bunch of ands. Did you review the implementation, Amit? So far everyone is okay with the API as something to work with for now. Lex -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Refactor SessionHandler and BrowserChannelClient to allow other OOPHM clients than HtmlUnit
Thanks for the patch. I agree with John that this is way too late for 2.0 at this point. I will get back to this for 2.1. Please file a public issue for this refactoring and attach the patch to that. Regards, Amit On Sat, Nov 28, 2009 at 6:41 PM, j...@google.com wrote: I really think this is too big a change to make between the RC and the final release. I would rather wait until after we ship 2.0 and spend a bit more time on it to clean up some of the other rough edges before getting it into trunk. http://gwt-code-reviews.appspot.com/114801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Remove distro-source/platform dirs
ping On Thu, Nov 19, 2009 at 12:04 PM, Amit Manjhi amitman...@google.com wrote: [Somehow, I got an error from the mail delivery system the first time. Apologies if you have already received it] Description: Remove distro-source/windows, distro-source/linux and distro-source/mac dirs and their contents. There is a libswt-webkit-carbon-3235.jnilib that looks safe to remove, since I could not find any references to it. I just wanted to confirm before removing it. Please review this at http://gwt-code-reviews.appspot.com/105803 Affected files: distro-source/linux/src/mozilla-hosted-browser.conf distro-source/mac/src/libswt-webkit-carbon-3235.jnilib Index: distro-source/linux/src/mozilla-hosted-browser.conf === --- distro-source/linux/src/mozilla-hosted-browser.conf (revision 7023) +++ distro-source/linux/src/mozilla-hosted-browser.conf (working copy) @@ -1,35 +0,0 @@ -# This file specifies a list of possible mozilla installations to load, in -# priority order. Non-existent paths are ignored, and the next location is -# searched. A best effort attempt is also made to ignore paths containing -# incompatible mozilla installations (for example, ones which use GTK1 instead -# of GTK2, which is required). The first apparently valid installation found -# will be used (although it could still fail later). -# -# Non-absolute paths are relative to the directory containing this file (that -# is, the GWT install directory). -# -# Non-system installations should contain a file named gwt-dl-loadorder.conf. -# The format of such a file is one shared library per line which dictates the -# order in which that installation's shared libraries must be loaded to prevent -# implicit loading of other libraries. In other words, no library should be -# loaded before its dependencies. This is to prevent the LD_LIBRARY_PATH or -# other system library load configuration from loading the default system -# version of the library instead of the version in the target installation. - - -# Prefer mozilla 1.7.13 if it exists, because it supports mouse wheel events. -# If you need mouse wheel events, you can install the distribution available at: -# -# http://google-web-toolkit.googlecode.com/svn/tools/redist/mozilla/mozilla-1.7.13.tar.gz -# -# However, this version may not run correctly on your system. If it doesn't, -# you can try installing a mozilla 1.7.13 built for your system. -mozilla-1.7.13 - -# This is the default mozilla that ships with GWT. -mozilla-1.7.12 - -# See if there are compatible mozilla distributions already installed. -/usr/lib/mozilla-1.7.13 -/usr/lib/mozilla-1.7.12 -/usr/lib/mozilla Index: distro-source/mac/src/libswt-webkit-carbon-3235.jnilib === Cannot display: file marked as a binary type. svn:mime-type = application/octet-stream -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove distro-source/platform dirs
Kelly and Scott are the only ones that have touched this file. Please comment if it is still necessary. On Fri, Nov 20, 2009 at 2:41 PM, j...@google.com wrote: mozilla-hosted-browser.conf is definitely not needed any more. I don't believe libswt is needed for Mac either, but then I am not sure why it was ever here in the first place rather than in jni/mac. http://gwt-code-reviews.appspot.com/105803 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove distro-source/platform dirs
Thanks! commited at r7085 (trunk) and r7086 (2.0) On Fri, Nov 20, 2009 at 2:52 PM, Scott Blum sco...@google.com wrote: Strange, I'd have thought we'd have pulled that file from /tools. Anyway, please kill it. On Fri, Nov 20, 2009 at 5:48 PM, Amit Manjhi amitman...@google.comwrote: Kelly and Scott are the only ones that have touched this file. Please comment if it is still necessary. On Fri, Nov 20, 2009 at 2:41 PM, j...@google.com wrote: mozilla-hosted-browser.conf is definitely not needed any more. I don't believe libswt is needed for Mac either, but then I am not sure why it was ever here in the first place rather than in jni/mac. http://gwt-code-reviews.appspot.com/105803 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Remove distro-source/platform dirs
[Somehow, I got an error from the mail delivery system the first time. Apologies if you have already received it] Description: Remove distro-source/windows, distro-source/linux and distro-source/mac dirs and their contents. There is a libswt-webkit-carbon-3235.jnilib that looks safe to remove, since I could not find any references to it. I just wanted to confirm before removing it. Please review this at http://gwt-code-reviews.appspot.com/105803 Affected files: distro-source/linux/src/mozilla-hosted-browser.conf distro-source/mac/src/libswt-webkit-carbon-3235.jnilib Index: distro-source/linux/src/mozilla-hosted-browser.conf === --- distro-source/linux/src/mozilla-hosted-browser.conf (revision 7023) +++ distro-source/linux/src/mozilla-hosted-browser.conf (working copy) @@ -1,35 +0,0 @@ -# This file specifies a list of possible mozilla installations to load, in -# priority order. Non-existent paths are ignored, and the next location is -# searched. A best effort attempt is also made to ignore paths containing -# incompatible mozilla installations (for example, ones which use GTK1 instead -# of GTK2, which is required). The first apparently valid installation found -# will be used (although it could still fail later). -# -# Non-absolute paths are relative to the directory containing this file (that -# is, the GWT install directory). -# -# Non-system installations should contain a file named gwt-dl-loadorder.conf. -# The format of such a file is one shared library per line which dictates the -# order in which that installation's shared libraries must be loaded to prevent -# implicit loading of other libraries. In other words, no library should be -# loaded before its dependencies. This is to prevent the LD_LIBRARY_PATH or -# other system library load configuration from loading the default system -# version of the library instead of the version in the target installation. - - -# Prefer mozilla 1.7.13 if it exists, because it supports mouse wheel events. -# If you need mouse wheel events, you can install the distribution available at: -# -# http://google-web-toolkit.googlecode.com/svn/tools/redist/mozilla/mozilla-1.7.13.tar.gz -# -# However, this version may not run correctly on your system. If it doesn't, -# you can try installing a mozilla 1.7.13 built for your system. -mozilla-1.7.13 - -# This is the default mozilla that ships with GWT. -mozilla-1.7.12 - -# See if there are compatible mozilla distributions already installed. -/usr/lib/mozilla-1.7.13 -/usr/lib/mozilla-1.7.12 -/usr/lib/mozilla Index: distro-source/mac/src/libswt-webkit-carbon-3235.jnilib === Cannot display: file marked as a binary type. svn:mime-type = application/octet-stream -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove startOnFirstThread references from webAppCreator
I tested it on Mac and on Linux. On Wed, Nov 18, 2009 at 11:28 AM, j...@google.com wrote: LGTM, but I haven't tested it http://gwt-code-reviews.appspot.com/103808 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove startOnFirstThread references from webAppCreator
thanks! commited as r6993 (trunk) and r6994 (2.0). On Wed, Nov 18, 2009 at 11:31 AM, Amit Manjhi amitman...@google.com wrote: I tested it on Mac and on Linux. On Wed, Nov 18, 2009 at 11:28 AM, j...@google.com wrote: LGTM, but I haven't tested it http://gwt-code-reviews.appspot.com/103808 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix thread safety issues in GwtLocaleFactoryImpl and LocaleUtils
http://gwt-code-reviews.appspot.com/102819/diff/1/7#newcode80 Line 80: On 2009/11/16 19:51:53, amitmanjhi wrote: use ConcurrentHashMap? Can avoid the lock. I don't believe so, as you have to hold the lock across containsKey/get/put. Wouldn't putIfAbsent(..) work? -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix thread safety issues in GwtLocaleFactoryImpl and LocaleUtils
LGTM On Mon, Nov 16, 2009 at 1:19 PM, John Tamplin j...@google.com wrote: On Mon, Nov 16, 2009 at 3:10 PM, Amit Manjhi amitman...@google.comwrote: use ConcurrentHashMap? Can avoid the lock. I don't believe so, as you have to hold the lock across containsKey/get/put. Wouldn't putIfAbsent(..) work? Since gets never block, it seems like this can't be used to ensure that all threads come up with the same GwtLocale instance for a given locale. The current implementation seems clearly correct, and the fact that we are having this discussion indicates using ConcurrentHashMap is not a clear win, especially given the time constraints. -- John A. Tamplin Software Engineer (GWT), Google -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RR: Introduces system property gwt.testMethodTimeoutMinutes
I agree with John. A system property is a global VM-wide option that seems like an overkill for this purpose. A command line option with sane defaults is probably the way to go. On Tue, Nov 3, 2009 at 3:50 PM, j...@google.com wrote: On 2009/11/03 23:49:14, Ray Ryan wrote: I imagine I'm not the only one who tweaks JUnitShell.TEST_METHOD_TIMEOUT_MILLIS when in the middle of long debug sessions. Wouldn't it be nice to be able to do so without touching the code? General questions: is a system property the right way to do this; is the property name okay; and is it okay to hit the system properties at class init time like that? Also deletes unneeded @SuppressWarnings I think this would be better as a JUnitShell command line option. http://gwt-code-reviews.appspot.com/89819 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Run tests multiple times with HtmlUnit to reduce false positives of failures
Yeah, the formatting is incorrect. The default formatting on my eclipse seems incorrect. I will upload a re-formatted patch, after fixing the eclipse issue. Thanks John. On Mon, Nov 2, 2009 at 2:09 PM, j...@google.com wrote: http://gwt-code-reviews.appspot.com/91806/diff/1/3 File user/src/com/google/gwt/junit/JUnitShell.java (right): http://gwt-code-reviews.appspot.com/91806/diff/1/3#newcode698 Line 698: getTopLogger().log(TreeLogger.ERROR, This formatting change doesn't look right (the previous version fit fine in 80 characters) and is unrelated to the actual change being made. http://gwt-code-reviews.appspot.com/91806/diff/1/4 File user/src/com/google/gwt/junit/RunStyle.java (right): http://gwt-code-reviews.appspot.com/91806/diff/1/4#newcode78 Line 78: * an error setting up for that mode These changes aren't related and I think are actually incorrect, since continuation lines should be indented 4 characters. http://gwt-code-reviews.appspot.com/91806 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: HostedMode relay to DevMode
LGTM, with a minor update to the WARN message, as we discussed on IM. Final text of the WARN message: The class com.google.gwt.dev.HostedMode is deprecated and will be removed -- use com.google.gwt.dev.DevMode instead for launching development mode On Thu, Oct 15, 2009 at 11:40 AM, j...@google.com wrote: Reviewers: amitmanjhi, Description: This is the class which will take the place of the existing HostedMode entry point (which will be renamed to DevMode). All it does is log a warning and run the real DevMode main. As noted in the code, this is suitable for testing in current trunk but things will be renamed before submit. Please review this at http://gwt-code-reviews.appspot.com/79802 Affected files: dev/core/src/com/google/gwt/dev/OldHostedMode.java Index: dev/core/src/com/google/gwt/dev/OldHostedMode.java === --- dev/core/src/com/google/gwt/dev/OldHostedMode.java (revision 0) +++ dev/core/src/com/google/gwt/dev/OldHostedMode.java (revision 0) @@ -0,0 +1,56 @@ +/* + * Copyright 2009 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the License); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an AS IS BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.gwt.dev; + +import com.google.gwt.core.ext.TreeLogger; + +/** + * Support old name for this entry point, logging a warning message before + * redirecting to the new name. + * + * TODO(jat): after trunk merge, rename this to HostedMode and HostedMode to + * DevMode. + */ +...@deprecated +public class OldHostedMode extends HostedMode { + + /** + * Support old name for this entry point, logging a warning message before + * redirecting to the new name. + * + * @param args command-line arguments + */ + public static void main(String[] args) { +OldHostedMode hostedMode = new OldHostedMode(); +if (new ArgProcessor(hostedMode.options).processArgs(args)) { + hostedMode.run(); + // Exit w/ success code. + System.exit(0); +} +// Exit w/ non-success code. +System.exit(-1); + } + + @Override + protected boolean doStartup() { +if (!super.doStartup()) { + return false; +} +getTopLogger().log(TreeLogger.WARN, +HostedMode is deprecated -- use DevMode instead); +return true; + } +} Property changes on: dev/core/src/com/google/gwt/dev/OldHostedMode.java ___ Added: svn:mime-type + text/x-java Added: svn:eol-style + native --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Add a HelpInfo link for old plugins
LGTM On Wed, Oct 14, 2009 at 12:05 PM, j...@google.com wrote: Reviewers: amitmanjhi, Description: A warning is printed when a really old plugin connects to the code server. This adds a HelpInfo link, which points at the UsingOOPHM web page for now, giving users information about getting a later plugin. In the future, this will be changed to a better location. Please review this at http://gwt-code-reviews.appspot.com/78819 Affected files: dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java Index: dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java === --- dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java (revision 6372) +++ dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java (working copy) @@ -16,12 +16,15 @@ package com.google.gwt.dev.shell; import com.google.gwt.core.ext.TreeLogger; +import com.google.gwt.core.ext.TreeLogger.HelpInfo; import com.google.gwt.dev.shell.JsValue.DispatchObject; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.net.MalformedURLException; import java.net.Socket; +import java.net.URL; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -233,8 +236,27 @@ moduleName = oldLoadModule.getModuleName(); userAgent = oldLoadModule.getUserAgent(); protocolVersion = 1; +HelpInfo helpInfo = new HelpInfo() { + @Override + public String getAnchorText() { +return UsingOOPHM wiki page; + } + + @Override + public URL getURL() { +try { + // TODO(jat): better landing page for more info + return new URL( + http://code.google.com/p/google-web-toolkit/wiki/UsingOOPHM;); +} catch (MalformedURLException e) { + // can't happen + return null; +} + } +}; logger.log(TreeLogger.WARN, Connection from old browser plugin -- -+ please upgrade to a later version for full functionality); ++ please upgrade to a later version for full functionality, null, +helpInfo); break; case CHECK_VERSIONS: String connectError = null; --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Remove @DoNotRunWith annotation of the passed test cases
Hi Frank, The farewellSwt branch is no longer active. John merged it back in trunk in r6366. Can you update this patch to be against trunk, post r6366? Also, can you double-check if the comments for tests that no longer use @DoNotRunWith need to be updated? Thanks, Amit On Tue, Oct 13, 2009 at 5:47 PM, f...@google.com wrote: Reviewers: amitmanjhi, Description: @DoNotRunWith({Platform.Htmlunit}) is the one gating the Htmlunit test cases that failed. Since bug fixes in GWT and HtmlUnit, some of these annotated test case are already passing. Need to remove these annotations. Please review this at http://gwt-code-reviews.appspot.com/78815 Affected files: user/test/com/google/gwt/dev/jjs/test/RunAsyncMetricsIntegrationTest.java user/test/com/google/gwt/dom/client/ElementTest.java user/test/com/google/gwt/dom/client/MapTests.java user/test/com/google/gwt/dom/client/NodeTest.java user/test/com/google/gwt/event/dom/client/DomEventTest.java user/test/com/google/gwt/uibinder/sample/client/UiBinderTest.java user/test/com/google/gwt/user/client/rpc/UnicodeEscapingTest.java user/test/com/google/gwt/user/client/ui/AnchorTest.java user/test/com/google/gwt/user/client/ui/ButtonTest.java user/test/com/google/gwt/user/client/ui/CheckBoxTest.java user/test/com/google/gwt/user/client/ui/CreateEventTest.java user/test/com/google/gwt/user/client/ui/DOMTest.java user/test/com/google/gwt/user/client/ui/DialogBoxTest.java user/test/com/google/gwt/user/client/ui/HistoryTest.java user/test/com/google/gwt/user/client/ui/ImageTest.java user/test/com/google/gwt/user/client/ui/MenuBarTest.java user/test/com/google/gwt/user/client/ui/PopupTest.java user/test/com/google/gwt/user/client/ui/SimpleRadioButtonTest.java user/test/com/google/gwt/user/client/ui/SuggestBoxTest.java user/test/com/google/gwt/user/client/ui/TreeTest.java user/test/com/google/gwt/xml/client/XMLTest.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
Announcing GWT 2.0 Milestone 1
Hi everyone, We are excited to release the first milestone build for GWT 2.0 today. This milestone provides early access (read: known to still be unfinished and buggy) to the various bits of core functionality that will be coming in GWT 2.0. Please download the bits from: http://code.google.com/p/google-web-toolkit/downloads/list?can=1q=2.0+Milestone+1 Things that are changing with GWT 2.0 that might otherwise be confusing without explanation * Terminology changes: We're going to start using the term development mode rather than the old term hosted mode. The term hosted mode was sometimes confusing to people, so we'll be using the more descriptive term from now on. For similar reasons, we'll be using the term production mode rather than web mode when referring to compiled script. * Changes to the distribution: Note that there's only one download, and it's no longer platform-specific. You download the same zip file for every development platform. This is made possible by the new plugin approach used to implement development mode (see below). The distribution file does not include the browser plugins themselves; those are downloaded separately the first time you use development mode in a browser that doesn't have the plugin installed. Functionality that will be coming in GWT 2.0 * In-Browser Development Mode: Prior to 2.0, GWT hosted mode provided a special-purpose hosted browser to debug your GWT code. In 2.0, the web page being debugged is viewed within a regular-old browser. Development mode is supported through the use of a native-code plugin for each browser. In other words, you can use development mode directly from Safari, Firefox, IE, and Chrome. * Code Splitting: Developer-guided code splitting allows you to chunk your GWT code into multiple fragments for faster startup. Imagine having to download a whole movie before being able to watch it. Well, that's what you have to do with most Ajax apps these days -- download the whole thing before using it. With code splitting, you can arrange to load just the minimum script needed to get the application running and the user interacting, while the rest of the app is downloaded as needed. * Declarative User Interface: GWT's UiBinder now allows you to create user interfaces mostly declaratively. Previously, widgets had to be created and assembled programmatically, requiring lots of code. Now, you can use XML to declare your UI, making the code more readable, easier to maintain, and faster to develop. The Mail sample has been updated to use the new declarative UI. * Bundling of resources (ClientBundle): GWT has shipped with ImageBundles since GWT v1.4, giving developers automatic spriting of images. ClientBundle generalizes this technique, bringing the power of combining and optimizing resources into one download to things like text files, CSS, and XML. This means fewer network round trips, which in turn can decrease application latency -- especially on mobile applications. * Using HtmlUnit for running GWT tests: GWT 2.0 no longer uses SWT or the old mozilla code (on linux) to run GWT tests. Instead, it uses HtmlUnit as the built-in browser. HtmlUnit is 100% Java. This means there is a single GWT distribution for linux, mac, and windows, and debugging GWT Tests in development mode can be done entirely in a Java debugger. Known issues * If you are planning to run the webAppCreator, i18nCreator, or the junitCreator scripts on Mac or Linux, please set their executable bits by doing a 'chmod +x *Creator' * Our HtmlUnit integration is still not complete. Additionally, HtmlUnit does not do layout. So tests can fail either because they exercise layout or they hit bugs due to incomplete integration. If you want such tests to be ignored on HtmlUnit, please annotate the test methods with @DoNotRunWith({Platform.Htmlunit}) * The Google Eclipse Plugin will only allow you to add GWT release directories that include a file with a name like gwt-dev-windows.jar. You can fool it by sym linking or copying gwt-dev.jar to the appropriate name. Breaking changes * The way arguments are passed to the GWT testing infrastructure has been revamped. There is now a consistent syntax to support arbitrary runstyles, including user-written with no changes to GWT. Though this does not affect common launch configs, some of the less common ones will need to be updated. For example, '-selenium FF3' has become '-runStyle selenium:FF3' As always, remember that GWT milestone builds like this are use-at- your-own-risk and we don't recommend it for production use. Please report any bugs you encounter to the GWT issue tracker (http:// code.google.com/p/google-web-toolkit/issues/list) after doing a quick search to see if your issue has already been reported. -- Amit Manjhi, on behalf of the Google Web Toolkit team --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Google Web Toolkit group. To post
[gwt-contrib] Announcing GWT 2.0 Milestone 1
Hi everyone, We are excited to release the first milestone build for GWT 2.0 today. This milestone provides early access (read: known to still be unfinished and buggy) to the various bits of core functionality that will be coming in GWT 2.0. Please download the bits from: http://code.google.com/p/google-web-toolkit/downloads/list?can=1q=2.0+Milestone+1 Things that are changing with GWT 2.0 that might otherwise be confusing without explanation * Terminology changes: We're going to start using the term development mode rather than the old term hosted mode. The term hosted mode was sometimes confusing to people, so we'll be using the more descriptive term from now on. For similar reasons, we'll be using the term production mode rather than web mode when referring to compiled script. * Changes to the distribution: Note that there's only one download, and it's no longer platform-specific. You download the same zip file for every development platform. This is made possible by the new plugin approach used to implement development mode (see below). The distribution file does not include the browser plugins themselves; those are downloaded separately the first time you use development mode in a browser that doesn't have the plugin installed. Functionality that will be coming in GWT 2.0 * In-Browser Development Mode: Prior to 2.0, GWT hosted mode provided a special-purpose hosted browser to debug your GWT code. In 2.0, the web page being debugged is viewed within a regular-old browser. Development mode is supported through the use of a native-code plugin for each browser. In other words, you can use development mode directly from Safari, Firefox, IE, and Chrome. * Code Splitting: Developer-guided code splitting allows you to chunk your GWT code into multiple fragments for faster startup. Imagine having to download a whole movie before being able to watch it. Well, that's what you have to do with most Ajax apps these days -- download the whole thing before using it. With code splitting, you can arrange to load just the minimum script needed to get the application running and the user interacting, while the rest of the app is downloaded as needed. * Declarative User Interface: GWT's UiBinder now allows you to create user interfaces mostly declaratively. Previously, widgets had to be created and assembled programmatically, requiring lots of code. Now, you can use XML to declare your UI, making the code more readable, easier to maintain, and faster to develop. The Mail sample has been updated to use the new declarative UI. * Bundling of resources (ClientBundle): GWT has shipped with ImageBundles since GWT v1.4, giving developers automatic spriting of images. ClientBundle generalizes this technique, bringing the power of combining and optimizing resources into one download to things like text files, CSS, and XML. This means fewer network round trips, which in turn can decrease application latency -- especially on mobile applications. * Using HtmlUnit for running GWT tests: GWT 2.0 no longer uses SWT or the old mozilla code (on linux) to run GWT tests. Instead, it uses HtmlUnit as the built-in browser. HtmlUnit is 100% Java. This means there is a single GWT distribution for linux, mac, and windows, and debugging GWT Tests in development mode can be done entirely in a Java debugger. Known issues * If you are planning to run the webAppCreator, i18nCreator, or the junitCreator scripts on Mac or Linux, please set their executable bits by doing a 'chmod +x *Creator' * Our HtmlUnit integration is still not complete. Additionally, HtmlUnit does not do layout. So tests can fail either because they exercise layout or they hit bugs due to incomplete integration. If you want such tests to be ignored on HtmlUnit, please annotate the test methods with @DoNotRunWith({Platform.Htmlunit}) * The Google Eclipse Plugin will only allow you to add GWT release directories that include a file with a name like gwt-dev-windows.jar. You can fool it by sym linking or copying gwt-dev.jar to the appropriate name. Breaking changes * The way arguments are passed to the GWT testing infrastructure has been revamped. There is now a consistent syntax to support arbitrary runstyles, including user-written with no changes to GWT. Though this does not affect common launch configs, some of the less common ones will need to be updated. For example, '-selenium FF3' has become '-runStyle selenium:FF3' As always, remember that GWT milestone builds like this are use-at- your-own-risk and we don't recommend it for production use. Please report any bugs you encounter to the GWT issue tracker (http:// code.google.com/p/google-web-toolkit/issues/list) after doing a quick search to see if your issue has already been reported. -- Amit Manjhi, on behalf of the Google Web Toolkit team --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: setInnerText fails in HtmlUnit
LGTM. Can you add a TODO to remove the annotation when we update to HtmlUnit-2.7 or above? On Fri, Oct 2, 2009 at 10:40 AM, jlaba...@google.com wrote: Reviewers: amitmanjhi, Description: Description: After r6263 (run HtmlUnit FF3 using gecko1_8 instead of gecko), an HtmlUnit bug was uncovered where it does not clear out all elements when setInnerText() is called. Supposedly this is a known issue that was fixed in the branch. This patch fixes it in trunk. Fix: Adding @DoNotRunWith(Platform.Htmlunit) to both tests. Please review this at http://gwt-code-reviews.appspot.com/75803 Affected files: user/test/com/google/gwt/dom/client/ElementTest.java user/test/com/google/gwt/user/client/ui/DOMTest.java Index: user/test/com/google/gwt/dom/client/ElementTest.java === --- user/test/com/google/gwt/dom/client/ElementTest.java(revision 6284) +++ user/test/com/google/gwt/dom/client/ElementTest.java(working copy) @@ -524,6 +524,7 @@ /** * innerText. */ + @DoNotRunWith(Platform.Htmlunit) public void testSetInnerText() { Document doc = Document.get(); Index: user/test/com/google/gwt/user/client/ui/DOMTest.java === --- user/test/com/google/gwt/user/client/ui/DOMTest.java(revision 6284) +++ user/test/com/google/gwt/user/client/ui/DOMTest.java(working copy) @@ -293,6 +293,7 @@ * Tests that {...@link DOM#setInnerText(Element, String)} works consistently * across browsers. */ + @DoNotRunWith(Platform.Htmlunit) public void testSetInnerText() { Element tableElem = DOM.createTable(); --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Fix GC handling in hosted mode
LGTM. Why use a TreeMap in RemoteObjectTable? On Thu, Sep 24, 2009 at 4:26 PM, j...@google.com wrote: Reviewers: amitmanjhi, Description: This corrects an issue that was there before and implements it on the client side of BrowserChannel, as needed by HtmlUnit in hosted mode. Please review this at http://gwt-code-reviews.appspot.com/71805 Affected files: dev/oophm/src/com/google/gwt/dev/shell/BrowserChannel.java dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelClient.java dev/oophm/src/com/google/gwt/dev/shell/BrowserChannelServer.java dev/oophm/src/com/google/gwt/dev/shell/JsValueOOPHM.java dev/oophm/src/com/google/gwt/dev/shell/ObjectsTable.java dev/oophm/src/com/google/gwt/dev/shell/OophmSessionHandler.java dev/oophm/src/com/google/gwt/dev/shell/RemoteObjectTable.java dev/oophm/src/com/google/gwt/dev/shell/ServerObjectsTable.java dev/oophm/test/com/google/gwt/dev/shell/BrowserChannelTest.java dev/oophm/test/com/google/gwt/dev/shell/RemoteObjectTableTest.java user/src/com/google/gwt/junit/RunStyle.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Fix GC handling in hosted mode
Yes. On Fri, Sep 25, 2009 at 10:35 AM, John Tamplin j...@google.com wrote: On Fri, Sep 25, 2009 at 1:22 PM, Amit Manjhi amitman...@google.comwrote: LGTM. Why use a TreeMap in RemoteObjectTable? I basically kept the same thing that Bob had there before, just moved it from ThreadLocals to an isolated synchronized class. I don't see any reason it needs to be ordered, so I will change it to a HashMap. Ok to commit with that change? -- John A. Tamplin Software Engineer (GWT), Google --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Ant file changes for removing swt and mac-os specific stuff
Nope, that can be inlined. Will commit with this change. Thanks! On Thu, Sep 24, 2009 at 12:57 PM, Freeland Abbott fabb...@google.comwrote: Also, is there a reason we can't discard junit.headless from common.ant.xml, rather than hardcoding it true? On Thu, Sep 24, 2009 at 3:53 PM, j...@google.com wrote: LGTM Reitveld is again acting up -- I couldn't actually view any of the files but the first and last, but I could get a diff of the whole patch to look at. http://gwt-code-reviews.appspot.com/71804/diff/1/2 File common.ant.xml (right): http://gwt-code-reviews.appspot.com/71804/diff/1/2#newcode185 Line 185: jvmarg line=-Xmx2048m / I don't know that we want to increase this in the build file. http://gwt-code-reviews.appspot.com/71804/diff/1/7 File user/build.xml (right): http://gwt-code-reviews.appspot.com/71804/diff/1/7#newcode145 Line 145: gwt.junit test.args=${test.args} test.out=${junit.out}/${build.host.platform}-hosted-mode test.cases=test.hosted.tests I assume this is temporary? http://gwt-code-reviews.appspot.com/71804 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Ant file changes for removing swt and mac-os specific stuff
The -batch module and bumping the heap changes are temporary. Actually, I will leave them out of the commit. On Thu, Sep 24, 2009 at 12:53 PM, j...@google.com wrote: LGTM Reitveld is again acting up -- I couldn't actually view any of the files but the first and last, but I could get a diff of the whole patch to look at. http://gwt-code-reviews.appspot.com/71804/diff/1/2 File common.ant.xml (right): http://gwt-code-reviews.appspot.com/71804/diff/1/2#newcode185 Line 185: jvmarg line=-Xmx2048m / I don't know that we want to increase this in the build file. http://gwt-code-reviews.appspot.com/71804/diff/1/7 File user/build.xml (right): http://gwt-code-reviews.appspot.com/71804/diff/1/7#newcode145 Line 145: gwt.junit test.args=${test.args} test.out=${junit.out}/${build.host.platform}-hosted-mode test.cases=test.hosted.tests I assume this is temporary? http://gwt-code-reviews.appspot.com/71804 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Add more HtmlUnit platforms/browsers testings
On Wed, Sep 9, 2009 at 11:19 AM, Frank Lin f...@google.com wrote: Hi Amit, On Wed, Sep 9, 2009 at 9:25 AM, amitman...@google.com wrote: Hi Frank, Instead of having individual ant commands for each browser emulation, it would be better to test all the browsers in parallel using -htmlunit FF3, FF2, IE6, IE7. I would suggest more ant targets rather let ant optimize the parallelism since blaze might be able to shard them better. We might need Freeland's expertise on that. When all browsers are specified, the parallelism is at the JVM level, not at the ant level. A separate HtmlUnit thread is started to emulate each browser. --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Add more HtmlUnit platforms/browsers testings
For now, it seems fine. It is the same issue when running multiple browsers simultaneously during remoteweb or selenium tests. On Wed, Sep 9, 2009 at 1:45 PM, Frank Lin f...@google.com wrote: Just one issue: is it okay to use test.out for all platforms if using '-htmlunit FF3, FF2, IE6, IE7'? test.out=${junit.out}/${build.host.platform}-htmlunit-web-mode On Wed, Sep 9, 2009 at 11:40 AM, Amit Manjhi amitman...@google.comwrote: On Wed, Sep 9, 2009 at 11:19 AM, Frank Lin f...@google.com wrote: Hi Amit, On Wed, Sep 9, 2009 at 9:25 AM, amitman...@google.com wrote: Hi Frank, Instead of having individual ant commands for each browser emulation, it would be better to test all the browsers in parallel using -htmlunit FF3, FF2, IE6, IE7. I would suggest more ant targets rather let ant optimize the parallelism since blaze might be able to shard them better. We might need Freeland's expertise on that. When all browsers are specified, the parallelism is at the JVM level, not at the ant level. A separate HtmlUnit thread is started to emulate each browser. --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Call webClient.closeAllWindows() in htmlunit code
Not while a test is running. Only when a test or a block of test ends does the GWTRunner ping again. On Thu, Aug 20, 2009 at 3:38 PM, Scott Blum sco...@google.com wrote: Does JUnit's GWTRunner keep pinging the server on a timer to see if more tests are wanted? On Thu, Aug 20, 2009 at 5:04 PM, kpro...@google.com wrote: In a discussion with Amit and Joel, we've determined that the tests that this will apply to do not use History tokens, so the repeated timeout won't apply. We couldn't think of any other cases where there would be lingering JavaScript jobs. TODO - we should revisit this issue at some point and come up with something more stable. On 2009/08/20 20:27:25, kathrin wrote: Hi Amit, I think this is the right direction! But I do think there's still a problem here, if I read the HtmlUnit documentation correctly. It says: - waitForBackgroundJavaScriptStartingBefore This method blocks until all background JavaScript tasks scheduled to start executing before (now + delayMillis) have finished executing. Background JavaScript tasks are JavaScript tasks scheduled for execution via window.setTimeout, window.setInterval or asynchronous XMLHttpRequest. ... Returns: the number of background JavaScript jobs still executing or waiting to be executed when this method returns; will be 0 if there are no jobs left to execute - Now what's happening here is that you wait x ms for the jobs to finish, but then there will at least be 1 still running (because GWT apps have a setTimeout(200?) that keeps setting a setTimeout(200) to check whether the history has changed. So unless I'm missing something, every time this method returns, it'll return a value 0, which means you'll just loop around until the test times out. Am I missing something? http://gwt-code-reviews.appspot.com/62802/diff/1/2 File user/src/com/google/gwt/junit/RunStyleHtmlUnit.java (right): http://gwt-code-reviews.appspot.com/62802/diff/1/2#newcode101 Line 101: + JAVASCRIPT_WAIT_TIME + ms); Rephrase a bit? Maybe say Waiting for (++count) javascript jobs for xxx ms http://gwt-code-reviews.appspot.com/62802 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: [google-web-toolkit] r5897 committed - Rolling back tools and trunk r5889 through 5892 due to ...
This case happens to be in the exception category. These were files that I had added a couple of days ago. And only my code depended on it. I found a way to use already existing libraries from tools instead of these libs. Since there was no other use for them, I deleted them after consulting with Ray. On Thu, Aug 6, 2009 at 10:33 AM, Scott Blum sco...@google.com wrote: Nothing should be deleted from tools/ ever. The only exceptions would be something added by mistake that no version of GWT depends on. On Wed, Aug 5, 2009 at 11:58 PM, codesite-nore...@google.com wrote: Revision: 5897 Author: rj...@google.com Date: Wed Aug 5 20:57:41 2009 Log: Rolling back tools and trunk r5889 through 5892 due to build break http://code.google.com/p/google-web-toolkit/source/detail?r=5897 Added: /tools/lib/xerces/serializer-2.7.1.jar /tools/lib/xerces/xercesImpl-2.8.1.jar /tools/lib/xerces/xml-apis-1.3.04.jar === --- /dev/null +++ /tools/lib/xerces/serializer-2.7.1.jar Wed Aug 5 20:57:41 2009 Binary file, no diff available. === --- /dev/null +++ /tools/lib/xerces/xercesImpl-2.8.1.jar Wed Aug 5 20:57:41 2009 File is too large to display a diff. === --- /dev/null +++ /tools/lib/xerces/xml-apis-1.3.04.jar Wed Aug 5 20:57:41 2009 Binary file, no diff available. --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Java changes for OOPHM wire protocol change
Yeah, the enhanced-for comment for assigning to array elements is dumb. My bad. Please feel free to submit after making changes. On Thu, Aug 6, 2009 at 12:46 PM, j...@google.com wrote: Thanks for the review. http://gwt-code-reviews.appspot.com/51835/diff/3001/3006 File dev/core/src/com/google/gwt/core/ext/linker/impl/hosted.html (right): http://gwt-code-reviews.appspot.com/51835/diff/3001/3006#newcode23 Line 23: $legacyHosted = true; On 2009/08/06 19:17:50, amitmanjhi wrote: Can we get rid of all code related to legacyHosted? Not yet, since people are still using it. When SWT is removed, this would be removed at the same time. http://gwt-code-reviews.appspot.com/51835/diff/3001/3006#newcode205 Line 205: } On 2009/08/06 19:17:50, amitmanjhi wrote: Can make this clearer using ASCII_EXCLAMATION = 33 and ASCII_TILDA = 127. The comment is then redundant. Ok. http://gwt-code-reviews.appspot.com/51835/diff/3001/3006#newcode238 Line 238: loadIframe( http://code.google.com/p/google-web-toolkit/wiki/TroubleshootingOOPHM;); On 2009/08/06 19:17:50, amitmanjhi wrote: Does this wiki not exist yet? I could not find it. No. I will create at least a template before committing. http://gwt-code-reviews.appspot.com/51835/diff/3001/3008 File dev/core/src/com/google/gwt/dev/SwtHostedModeBase.java (right): http://gwt-code-reviews.appspot.com/51835/diff/3001/3008#newcode80 Line 80: String remoteEndpoint) throws UnableToCompleteException { On 2009/08/06 19:17:50, amitmanjhi wrote: a more descriptive name than string? Missing javadoc on a public method. If this class will be removed in 2.0, I suppose you can leave it as such. The auto-refactor of the implementing class did this, and I have already fixed it locally. In fact, it is url, sessionKey rather than sessionKey, string. http://gwt-code-reviews.appspot.com/51835/diff/3001/3003 File dev/oophm/src/com/google/gwt/dev/shell/BrowserChannel.java (right): http://gwt-code-reviews.appspot.com/51835/diff/3001/3003#newcode621 Line 621: } On 2009/08/06 19:17:50, amitmanjhi wrote: Use enhanced-for loop? How would the enhanced for work here? I am assigning to the array elements. http://gwt-code-reviews.appspot.com/51835/diff/3001/3003#newcode643 Line 643: for (int i = 0; i n; ++i) { On 2009/08/06 19:17:50, amitmanjhi wrote: Use enhanced-for loop? Ok. http://gwt-code-reviews.appspot.com/51835/diff/3001/3003#newcode662 Line 662: } On 2009/08/06 19:17:50, amitmanjhi wrote: Use enhanced-for loop? Again, doesn't work assigning to the array. http://gwt-code-reviews.appspot.com/51835 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: patch to enable web tests using HtmlUnit
Thanks Scott. Commited the changes at r5844 with a fix the first thing and a TODO for the second thing. On Thu, Jul 30, 2009 at 10:48 AM, sco...@google.com wrote: LGTM with nits. http://gwt-code-reviews.appspot.com/54809/diff/1096/1130 File user/src/com/google/gwt/junit/JUnitShell.java (right): http://gwt-code-reviews.appspot.com/54809/diff/1096/1130#newcode220 Line 220: + e.g. IE6,IE7,FF2,FF3...; Could the exact list be gotten from RunStyleHtmlUnit? Then you don't need to update this message if the set of browsers changes. http://gwt-code-reviews.appspot.com/54809/diff/1096/1129 File user/src/com/google/gwt/junit/RunStyleHtmlUnit.java (right): http://gwt-code-reviews.appspot.com/54809/diff/1096/1129#newcode149 Line 149: threads.add(hut); Is there any way to, say, pause here waiting for the other thread to get to an okay state that would could call a successful launch? Might be nice to detect report errors early. http://gwt-code-reviews.appspot.com/54809 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Add emma_ant.jar to tools directory
emma.jar includes our modifications to emma. That is why redist was chosen instead of lib. The version is included in the name of zip file: emma-2.0.5312-src.zip When the zip file is expanded, it just includes 'emma.jar' with no version number. So, it made sense to make emma with our patch available under the same name. On Thu, Jul 23, 2009 at 11:53 AM, Scott Blum sco...@google.com wrote: Also, don't we need a version on emma_ant.jar? There should have been one put on emma.jar, but it looks like an oversight. On Thu, Jul 23, 2009 at 2:51 PM, Scott Blum sco...@google.com wrote: Why does emma live in redist/ instead of /lib anyway? On Thu, Jul 23, 2009 at 2:47 PM, jlaba...@google.com wrote: Reviewers: bruce, Description: This patch adds the emma_ant jar to our tools directory so the GWT build file can use it to generate code coverage data. We already include emma.jar. A subsequent patch to trunk will update the build files to add code coverage stats. Please review this at http://gwt-code-reviews.appspot.com/51812 Affected files: redist/emma/README.txt redist/emma/emma_ant.jar Index: redist/emma/README.txt === --- redist/emma/README.txt (revision 5777) +++ redist/emma/README.txt (working copy) @@ -9,6 +9,9 @@ emma.jar: the jar file produced by the patched emma. +emma_ant.jar: the jar file containing custom EMMA ant tasks that can be +downloaded from sourceforge.net. + com.mountainminds.eclemma.core_1.3.2.jar : eclemma jar containing the updated emma.jar Index: redist/emma/emma_ant.jar === Cannot display: file marked as a binary type. svn:mime-type = application/octet-stream Property changes on: redist/emma/emma_ant.jar ___ Name: svn:mime-type + application/octet-stream --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: apicheck should run on Windows
Good catch. LGTM On Fri, Jul 10, 2009 at 9:40 PM, fabb...@google.com wrote: Reviewers: amitmanjhi, Description: the old form got confused by a refJar path of 'C:\GWT\tools\api-checker\reference\gwt-dev-modified.jar:C:\GWT\tools\api-checker\reference\gwt-user-modified.jar' because splitting on colons was wrong. Worse, ant thought that it had succeeded because an arg-parsing error was caught, help printed, and ultimately 0 exited. The new form splits on the path.separator system propery instead, uses ant's arg path=.../ to input either a : or ; separated value depending on OS, and catches arg parse failures as non-zero return. Please review this at http://gwt-code-reviews.appspot.com/51801 Affected files: build.xml tools/api-checker/src/com/google/gwt/tools/apichecker/ApiCompatibilityChecker.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: support for internationalized svninfo
I also tried this patch. Build works great. But, 'ant test -dgwt.svnrev=...@yyy' fails because com.google.gwt.ant.taskdefs.SvnInfoTest fails. If the ant property is defined, should the SvnInfoTest be disabled as well? Regards, Amit On Wed, Jul 8, 2009 at 3:22 AM, chassa...@gmail.com wrote: hi, Today I updated my gwt with the trunk (rev 5687). I removed my changes (uncomment lines). I applied the patch on the file : ./build-tools/ant-gwt/src/com/google/gwt/ant/taskdefs/SvnInfo.java Now it works. I didn't need to create a local.ant.properties file. Thanks ! Best regards, Seb http://gwt-code-reviews.appspot.com/48807 --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Specify the path for translatable code explicitly in new apps created by webAppCreator
Thanks Scott and Ray. Commited at r. On Fri, Jun 12, 2009 at 4:11 PM, Ray Ryan rj...@google.com wrote: LGTM++ On Fri, Jun 12, 2009 at 4:08 PM, Scott Blum sco...@google.com wrote: LGTM On Fri, Jun 12, 2009 at 6:49 PM, amitman...@google.com wrote: Reviewers: Ray Ryan, scottb, Description: It will be less confusing for new users. Anyone see any downsides? Please review this at http://gwt-code-reviews.appspot.com/39803 Affected files: user/src/com/google/gwt/user/tools/Module.gwt.xmlsrc Index: user/src/com/google/gwt/user/tools/Module.gwt.xmlsrc === --- user/src/com/google/gwt/user/tools/Module.gwt.xmlsrc(revision 5552) +++ user/src/com/google/gwt/user/tools/Module.gwt.xmlsrc(working copy) @@ -14,4 +14,8 @@ !-- Specify the app entry point class. -- entry-point class='@clientpacka...@moduleshortname'/ + + !-- Specify the paths for translatable code-- + source path='client'/ + /module --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Speed up compilation by rewriting for loops to avoid iterators
I completely agree with Scott here. It's hard to believe that there will be a consistent 3% improvement just by using an explicit index. The change is very much compiler/JVM dependent. And even if there were the case currently, the next compiler/JVM could very easily fix this. Since I was surprised by the result, I wrote up a simple micro-benchmark that repeats this test with Integer lists of varying sizes. I ran it with openjdk and did not see *any* performance difference. I have attached the quick-and-dirty code and included the sample results at the end. Amit = Here are sample results: java -cp . -Xmx1500M com.test.Test iteration with 1000 elements long way: 1, short way: 1 long way: 1, short way: 1 long way: 2, short way: 3 long way: 1, short way: 3 long way: 2, short way: 5 long way: 9, short way: 7 long way: 1, short way: 2 long way: 10, short way: 2 long way: 0, short way: 2 long way: 0, short way: 1 iteration with 100 elements long way: 11, short way: 12 long way: 14, short way: 11 long way: 20, short way: 20 long way: 20, short way: 22 long way: 27, short way: 27 long way: 28, short way: 30 long way: 38, short way: 42 long way: 37, short way: 42 long way: 45, short way: 45 long way: 46, short way: 49 iteration with 1000 elements long way: 124, short way: 124 long way: 125, short way: 125 long way: 280, short way: 262 long way: 282, short way: 262 long way: 349, short way: 345 long way: 348, short way: 344 long way: 529, short way: 499 long way: 566, short way: 663 long way: 530, short way: 508 long way: 530, short way: 508 On Thu, Jun 4, 2009 at 1:58 PM, Aaron Steele eightyste...@gmail.com wrote: Do'h! Yeah, using the name 'ints' probably wasn't a good choice here. Looks like I should re-read Item 56: Adhere to generally accepted naming conventions. :) On Thu, Jun 4, 2009 at 1:09 PM, Alex Rudnick a...@google.com wrote: Yeesh, pardon. That's an ArrayList called ints of Integers, not containing ints. I retract that statement! On Thu, Jun 4, 2009 at 4:04 PM, Alex Rudnick a...@google.com wrote: Sounds like boxing/unboxing overhead, in that case! What if you tried that with an array of native ints? On Thu, Jun 4, 2009 at 3:47 PM, Aaron Steele eightyste...@gmail.com wrote: So item 46 in Effective Java says that there shouldn't be a performance penalty using the nice for loops. But the following test in Eclipse on my machine (MacBook Pro, Intel Core Duo, 2.16 GHz) shows a performance penalty. Given an ArrayList called ints with 1 million Integers, this takes 31 milliseconds: for (int i = 0, size = ints.size(); i size; i++) ints.get(i).intValue(); And this takes 76 milliseconds: for (Integer i : ints) i.intValue(); What am I missing? Probably just some super naive testing on my part. :) -- Alex Rudnick swe, gwt, atl -- Sent from Piedmont, CA, United States --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~--- /* * Copyright 2008 Google Inc. * * Licensed under the Apache License, Version 2.0 (the License); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an AS IS BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the * License for the specific language governing permissions and limitations under * the License. */ package com.test; import java.util.ArrayList; import java.util.List; import java.util.Random; /** * An implementation for Test. */ public class Test { static final int ITERATIONS = 5; public static void main(String args[]) { ListInteger integerList = new ArrayListInteger(); for (int maxSize : new int[] {1000, 1000 * 1000, 1000 * 1000 * 10}) { long start = 0, shortWay = 0, longWay = 0; System.out.println(\n\niteration with + maxSize + elements); for (int i = 0; i ITERATIONS; i++) { constructList(integerList, maxSize); start = System.currentTimeMillis(); findMaxLongWay(integerList); longWay = System.currentTimeMillis() - start; findMaxShortWay(integerList); shortWay = System.currentTimeMillis() - start - longWay; System.out.println(long way: + longWay + , short way: + shortWay); // repeat in reverse order to get rid of memory/caching effects start = System.currentTimeMillis(); findMaxShortWay(integerList); shortWay = System.currentTimeMillis() - start; findMaxLongWay(integerList); longWay = System.currentTimeMillis() - start - shortWay; System.out.println(long way: + longWay + , short way: + shortWay);
[gwt-contrib] patch to fix issue 2979
Hi John/Scott, Please review this simple patch that fixes issue 2979. Regards, Amit --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~--- Index: dev/windows/src/org/eclipse/swt/browser/WebSite.java === --- dev/windows/src/org/eclipse/swt/browser/WebSite.java (revision 5360) +++ dev/windows/src/org/eclipse/swt/browser/WebSite.java (working copy) @@ -38,8 +38,9 @@ */ public boolean startGears() { // Get the classID of the Gears BHO. + GUID appClsid = null; try { -GUID appClsid = getClassID(gears.BHO); +appClsid = getClassID(gears.BHO); if (appClsid == null) { return false; }
[gwt-contrib] Re: patch to fix issue 2979
Thanks! Seems like it never worked. Commited as r5364. On Wed, May 13, 2009 at 1:03 PM, Scott Blum sco...@google.com wrote: LGTM, but how could this have ever worked? On Wed, May 13, 2009 at 3:32 PM, amitman...@google.com wrote: Reviewers: jat, scottb, Description: Hi John/Scott, Please review this simple patch that fixes issue 2979. Regards, Amit Please review this at http://gwt-code-reviews.appspot.com/34807 Affected files: dev/windows/src/org/eclipse/swt/browser/WebSite.java Index: dev/windows/src/org/eclipse/swt/browser/WebSite.java === --- dev/windows/src/org/eclipse/swt/browser/WebSite.java(revision 5360) +++ dev/windows/src/org/eclipse/swt/browser/WebSite.java(working copy) @@ -38,8 +38,9 @@ */ public boolean startGears() { // Get the classID of the Gears BHO. + GUID appClsid = null; try { -GUID appClsid = getClassID(gears.BHO); +appClsid = getClassID(gears.BHO); if (appClsid == null) { return false; } --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Fix for issue 3057
Thanks! Commited as r5366. On Wed, May 13, 2009 at 2:24 PM, Scott Blum sco...@google.com wrote: LGTM On Wed, May 13, 2009 at 5:00 PM, amitman...@google.com wrote: Reviewers: jat, scottb, Description: Hi John/Scott, Please review this patch that fixes issue 3057. I also added testcases and verified that the testcases pass in both web and hosted mode. Thanks, Amit Please review this at http://gwt-code-reviews.appspot.com/33813 Affected files: user/super/com/google/gwt/emul/java/util/AbstractList.java user/test/com/google/gwt/emultest/java/util/AbstractSequentialListTest.java --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Fix for issue 3422
The last import in TreeMapStringStringTest.java is unnecessary. Will remove it when I commit the patch. On Wed, May 13, 2009 at 4:15 PM, amitman...@google.com wrote: Reviewers: jat, scottb, Description: Simple fix and accompanying testcase for toString() method in TreeMap.Entry Please review this at http://gwt-code-reviews.appspot.com/33814 Affected files: user/super/com/google/gwt/emul/java/util/TreeMap.java user/test/com/google/gwt/emultest/java/util/TreeMapStringStringTest.java Index: user/super/com/google/gwt/emul/java/util/TreeMap.java --- user/super/com/google/gwt/emul/java/util/TreeMap.java (revision 3877) +++ user/super/com/google/gwt/emul/java/util/TreeMap.java (working copy) @@ -234,7 +234,8 @@ public class TreeMapK, V extends AbstractMapK, V implements @Override public String toString() { - return (isRed ? R: : B: ) + key + = + value; + // for compatibility with the real Jre: issue 3422 + return key + = + value; } } Index: user/test/com/google/gwt/emultest/java/util/TreeMapStringStringTest.java --- user/test/com/google/gwt/emultest/java/util/TreeMapStringStringTest.java (revision 3589) +++ user/test/com/google/gwt/emultest/java/util/TreeMapStringStringTest.java (working copy) @@ -16,9 +16,12 @@ package com.google.gwt.emultest.java.util; import java.util.Iterator; +import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; import java.util.SortedMap; +import java.util.TreeMap; +import java.util.Map.Entry; /** * Tests codeTreeMap/code with Strings and the natural comparator. @@ -118,6 +121,15 @@ public class TreeMapStringStringTest extends TreeMapTestString, String { assertEquals(lastKey, dd, subMap.lastKey()); } + // checks for compatibility with real Jre's Entry.toString(): issue 3422 + public void testTreeMapEntryToString() { +MapString, String treeMap = new TreeMapString, String(); +treeMap.put(bar, barValue); + +assertEquals(bar=barValue, +treeMap.entrySet().iterator().next().toString()); + } + @Override protected Object getConflictingKey() { return new Integer(1); --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---