Thanks for the review. Committed as r875. On Fri, Oct 10, 2008 at 9:14 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote: > 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 >
-- Eric Z. Ayers - GWT Team - Atlanta, GA USA http://code.google.com/webtoolkit/ --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
