LGTM On Thu, Oct 9, 2008 at 12:07 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
> +gwtc > > I missed the comments about the UI. The re-attached patch includes the UI > updates including a description of using the animation and new lables for > the radio buttons. I also attached a screenshot. > > > > 2008/10/8 Eric Ayers <[EMAIL PROTECTED]> > > Updated patch attached. >> >> On Wed, Oct 8, 2008 at 10:02 AM, Miguel Méndez <[EMAIL PROTECTED]>wrote: >> >>> On Mon, Oct 6, 2008 at 2:48 PM, Eric Ayers <[EMAIL PROTECTED]> wrote: >>> >>>> Hello Miguel >>>> >>>> Attached is a demo for the WorkerPool class that I would like for you to >>>> review. >>>> >>>> The demo is a bit more rich than the demos for the other Gears >>>> features. I intend to add documentation for a simple worker pool tutorial >>>> in the Getting Started guide that will show a simple version of how to use >>>> WorkerPool, so I felt justified in making a more complex demo. >>>> >>>> The demo shows how the interface is interactive when a worker pool >>>> thread is running by giving a little animation to play with (and the >>>> results >>>> are printed out as they come out). >>>> >>>> Also, there is a small enhancement to WorkerPool.sendMessage() part of >>>> the attached patch that allows the user to pass a JSO as the message. It >>>> is >>>> not necessary for the demo and I can't recall if I already sent it to you >>>> for review or not. >>>> >>>> >>>> M gears/src/com/google/gwt/gears/client/workerpool/WorkerPool.java >>>> >>> I did not realize that they had extended the API to allow the passing of >>> boolean, >>> string, number, array, object, and Blobs. Doesn't this mean that the >>> WorkerPoolMessageHandler.MessageEvent >>> class will need to be updated? It mirrors the messageObject parameter to >>> the JS API's onmessage callback, but now messageObject.body should really be >>> a JSO and we should expose messageObject.text. You'd need to figure out how >>> to deal with the case where the caller passed a string because >>> messageObject.body could contain a string in pure JS but in the Java wrapper >>> String and JSO's are not compatible. >>> >> >> I am going to pull this change out of the patch and submit it separately >> after I consider these changes. I'll need to beef up the unit tests as well. >> >> >>> >>> A samples/workerpool/launch-scripts/linux/WorkerPoolDemo-shell >>>> >>> Did not review. >>> >>> >>>> A samples/workerpool/launch-scripts/linux/WorkerPoolDemo-compile >>>> >>> Did not review. >>> >>> >>>> >>>> A samples/workerpool/launch-scripts/mac/WorkerPoolDemo-shell >>>> >>> LG >>> >>> >>>> A samples/workerpool/launch-scripts/mac/WorkerPoolDemo-compile >>>> >>> LG >>> >>> >>>> A >>>> samples/workerpool/launch-scripts/windows/WorkerPoolDemo-shell.cmd >>>> >>> Did not review. >>> >>> A >>>> samples/workerpool/launch-scripts/windows/WorkerPoolDemo-compile.cmd >>>> >>> Did not review. >>> >>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/WorkerPoolDemo.gwt.xml >>>> >>> LG >>> >>> >>>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/client/WorkerPoolDemoNoGears.java >>>> >>> LG >>> >>> >>>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/client/WorkerPoolDemo.java >>>> >>> LG - You should add some javadoc to the doSync*, doAsync*, and the >>> stopCalculation methods. Also, on line 134 you may want a comment that >>> explains why we use a deferred command to trigger either computation. >>> >> updated. >> >> >>> >>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/client/AnimationToy.java >>>> >>> LG >>> >>> >>>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/client/PiSpigot.java >>>> >>> LG - The name did not make sense to me until I read the Java comment. >>> >>> >>>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/public/gears_init.js >>>> >>> You should not need to include the startup script since you will inherit >>> it from Gears.gwt.xml. >>> >> Right. That was in there for a test fixture I had to test the JS code. >> removed. >> >> >>> >>>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/public/pi_spigot_worker.js >>>> >>> Nice JS code! What is the plan for showing a worker thread in GWT? >>> >> >> The plan is to get this demo finished, and then see if I can duplicate it >> with the Java implementation used for the synchronous calculation. >> >> >>> >>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/public/gwtLogoThumb.png >>>> >>> The file was not included in the patch; it prevented me from seeing the >>> animation aspect of the demo. Could you send it to me? >>> >> (done) >> >> >>> >>> >>>> A >>>> samples/workerpool/src/com/google/gwt/gears/sample/workerpool/public/WorkerPoolDemo.html >>>> >>> LG >>> >>> >>>> >>>> A samples/workerpool/build.xml >>>> >>> LG >>> >>> >>>> M samples/build.xml >>>> >>> LG >>> >>> -- >>> Miguel >>> >> >> >> >> -- >> Eric Z. Ayers - GWT Team - Atlanta, GA USA >> http://code.google.com/webtoolkit/ >> > > > > -- > Eric Z. Ayers - GWT Team - Atlanta, GA USA > http://code.google.com/webtoolkit/ > -- Miguel --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
