Nice, thanks. This really doesn't feel so bad to me. Just some notes below about comments to add.
http://gwt-code-reviews.appspot.com/1521803/diff/1/samples/mobilewebapp/pom.xml File samples/mobilewebapp/pom.xml (right): http://gwt-code-reviews.appspot.com/1521803/diff/1/samples/mobilewebapp/pom.xml#newcode73 samples/mobilewebapp/pom.xml:73: </dependency> Could you add a comment to this effect? On 2011/08/15 22:04:18, rchandia wrote:
On 2011/08/12 22:38:29, rjrjr wrote: > Are user and dev really provided? I thought GPE backs off and relies
on maven
to > download them. GPE does defer to the GWT maven plugin to silently get gwt-dev.jar,
but in this
case we want it available to javac to compile the AppCacheLinker. Of
course we
do not want it available ar runtime nor to gwtc (because gwtc gets it
in its own
way), but Maven is designed to be coarse grained in this regard. GWTC
complains,
but this still works.
The scope "provided" means get it, use it to compile (javac), but not
when
packaging nor testing. It should prevent gwt-dev.jar from reaching the
war; but
a bug in the maven plugin makes it reach the war nevertheless so we
forcibly
delete it later.
http://gwt-code-reviews.appspot.com/1521803/diff/1/samples/mobilewebapp/pom.xml#newcode182 samples/mobilewebapp/pom.xml:182: <!-- TODO: Who is using this? Just GAE? Is anyone, really? --> It would be good to add a comment saying so. Seems odd that the mvn gae plugin doesn't handle this for you. On 2011/08/15 22:04:18, rchandia wrote:
On 2011/08/12 22:38:29, rjrjr wrote: > etc. Yes, this one is used by App Engine
http://gwt-code-reviews.appspot.com/1521803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/server/domain/Task.java File samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/server/domain/Task.java (right): http://gwt-code-reviews.appspot.com/1521803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/server/domain/Task.java#newcode50 samples/mobilewebapp/src/main/java/com/google/gwt/sample/mobilewebapp/server/domain/Task.java:50: populateDatastore(); Could you add a TODO? Maybe even a pointer to David's sample. On 2011/08/15 22:04:18, rchandia wrote:
On 2011/08/12 22:38:29, rjrjr wrote: > populateDatastore uses its own emf. Does the one in findAllTasks
actually see
> the changes populateDatastore makes -- did you actually see the
samples show
up? Done. EMF is now used as a singleton.
The next step will be to use Locators and ServiceLocators to provide a
more
entrerprisey sample. In a future patch, though.
http://gwt-code-reviews.appspot.com/1521803/diff/5002/samples/mobilewebapp/pom.xml File samples/mobilewebapp/pom.xml (right): http://gwt-code-reviews.appspot.com/1521803/diff/5002/samples/mobilewebapp/pom.xml#newcode308 samples/mobilewebapp/pom.xml:308: <!-- Don't deploy gwt-user jar to GAE --> Another good spot for a comment explaining why we have to do this, and musing on what we might do instead in future. Also, it looks like you're stripping gwt-dev as well, no? http://gwt-code-reviews.appspot.com/1521803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
