LGTM, except some comments below.

http://gwt-code-reviews.appspot.com/328801/diff/1/3
File
bikeshed/src/com/google/gwt/sample/expenses/gwt/place/ApplicationListPlace.java
(right):

http://gwt-code-reviews.appspot.com/328801/diff/1/3#newcode25
bikeshed/src/com/google/gwt/sample/expenses/gwt/place/ApplicationListPlace.java:25:
public class ApplicationListPlace extends ExpensesPlace {
ExpensesListPlace instead of ApplicationListPlace?

http://gwt-code-reviews.appspot.com/328801/diff/1/4
File
bikeshed/src/com/google/gwt/sample/expenses/gwt/place/ApplicationPlaces.java
(right):

http://gwt-code-reviews.appspot.com/328801/diff/1/4#newcode26
bikeshed/src/com/google/gwt/sample/expenses/gwt/place/ApplicationPlaces.java:26:
public class ApplicationPlaces {
Rename ExpensesPlace instead of ApplicationPlaces and rename the current
ExpensesPlace to ExpensesKeyPlace?

http://gwt-code-reviews.appspot.com/328801/diff/1/4#newcode33
bikeshed/src/com/google/gwt/sample/expenses/gwt/place/ApplicationPlaces.java:33:
public <K extends ExpensesKey<K>> ActionCell.Delegate<Values<K>>
getDetailsGofer() {
Use getDetailsHelper instead of getDetailsGofer?

http://gwt-code-reviews.appspot.com/328801/diff/1/4#newcode36
bikeshed/src/com/google/gwt/sample/expenses/gwt/place/ApplicationPlaces.java:36:
goToDetailsFor(object);
inline?

http://gwt-code-reviews.appspot.com/328801/diff/1/4#newcode44
bikeshed/src/com/google/gwt/sample/expenses/gwt/place/ApplicationPlaces.java:44:
goToEditorFor(object);
inline?

http://gwt-code-reviews.appspot.com/328801/diff/1/13
File
bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/AllEmployeesRequester.java
(right):

http://gwt-code-reviews.appspot.com/328801/diff/1/13#newcode42
bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/AllEmployeesRequester.java:42:
view.getProperties()).to(view).fire();
Add a TODO here that start and length are not currently being used?

http://gwt-code-reviews.appspot.com/328801/diff/1/15
File
bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListView.java
(right):

http://gwt-code-reviews.appspot.com/328801/diff/1/15#newcode73
bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListView.java:73:
return properties;
cache this value? Similarly for ReportListView.

http://gwt-code-reviews.appspot.com/328801/diff/1/16
File
bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/AllReportsRequester.java
(right):

http://gwt-code-reviews.appspot.com/328801/diff/1/16#newcode44
bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/AllReportsRequester.java:44:
}
Add a TODO that start and length are not being used.

http://gwt-code-reviews.appspot.com/328801/diff/1/19
File bikeshed/src/com/google/gwt/valuestore/client/ListModelAdapter.java
(right):

http://gwt-code-reviews.appspot.com/328801/diff/1/19#newcode41
bikeshed/src/com/google/gwt/valuestore/client/ListModelAdapter.java:41:
}
aha! nice fix.

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

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

Reply via email to