LGTM, the comments are mostly questions to make sure that I understand
what the code is intended to do.


http://gwt-code-reviews.appspot.com/755801/diff/5001/6002
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/CalendarProvider.java
(right):

http://gwt-code-reviews.appspot.com/755801/diff/5001/6002#newcode50
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/CalendarProvider.java:50:

Extra whitespace.  I remember having to mess with my checkstyle settings
to convince eclipse it wanted to check stuff in the dynatablerf sample.

http://gwt-code-reviews.appspot.com/755801/diff/5001/6002#newcode78
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/CalendarProvider.java:78:
if (response.size() > 0) {
This if() block would get moved into the dismiss-edit-dialog, right?

http://gwt-code-reviews.appspot.com/755801/diff/5001/6006
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java
(right):

http://gwt-code-reviews.appspot.com/755801/diff/5001/6006#newcode24
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java:24:
public static Person findPerson(Long id) {
Is this another magic method?

http://gwt-code-reviews.appspot.com/755801/diff/5001/6007
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/SchoolCalendarService.java
(right):

http://gwt-code-reviews.appspot.com/755801/diff/5001/6007#newcode91
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/SchoolCalendarService.java:91:
public static Person findPerson(Long id) {
Magic?

http://gwt-code-reviews.appspot.com/755801/diff/5001/6007#newcode99
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/SchoolCalendarService.java:99:
person.setVersion(person.getVersion() + 1);
Is this version stuff important to the sample, or a holdover from a
JPA-style sample?

http://gwt-code-reviews.appspot.com/755801/diff/5001/6008
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/shared/DynaTableRequestFactory.java
(right):

http://gwt-code-reviews.appspot.com/755801/diff/5001/6008#newcode37
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/shared/DynaTableRequestFactory.java:37:
@Instance
This means that the method should be invoked on the instance of the
first argument?

http://gwt-code-reviews.appspot.com/755801/diff/5001/6008#newcode50
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/shared/DynaTableRequestFactory.java:50:
PersonRequest personRequest();
So the PersonRequest interface is almost like a vtable for remote calls?
 It just has a bunch of method stubs to queue up operations on?

http://gwt-code-reviews.appspot.com/755801/show

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

Reply via email to