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.

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.

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.


> 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?

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?


> 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

--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---

Reply via email to