I have finished my part of the code review. Please see comments bellow. -Thomas http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... File client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/model/ApplicationModel.java (right): http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/model/ApplicationModel.java:133: return ("true".equals(get(ApplicationKey.S_SH_WTD_ENABLED.name()))); May want to create a final static for the "true" string or use the Boolean.TRUE.toString ... http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... File client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/view/panel/MultiGradeContentPanel.java (right): http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/view/panel/MultiGradeContentPanel.java:468: if (showWeightedString != null && showWeightedString.trim().equalsIgnoreCase("true")) { Use Boolean.TRUE.toString() http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/view/panel/MultiGradeContentPanel.java:1104: .append("/") Create static finals for constant strings http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/view/panel/MultiGradeContentPanel.java:1169: if (showWeightedString != null && showWeightedString.trim().equalsIgnoreCase("true")) { Use Boolean.TRUE.toString() http://codereview.appspot.com/4172043/diff/1/server/src/java/org/sakaiproject... File server/src/java/org/sakaiproject/gradebook/gwt/sakai/calculations/GradeCalculationsImpl.java (right): http://codereview.appspot.com/4172043/diff/1/server/src/java/org/sakaiproject... server/src/java/org/sakaiproject/gradebook/gwt/sakai/calculations/GradeCalculationsImpl.java:2: NOTE: I checked that the calculations use the new BigDeciamlWrapper class. http://codereview.appspot.com/4172043/diff/1/server/src/java/org/sakaiproject... File server/src/java/org/sakaiproject/gradebook/gwt/sakai/rest/resource/Roster.java (right): http://codereview.appspot.com/4172043/diff/1/server/src/java/org/sakaiproject... server/src/java/org/sakaiproject/gradebook/gwt/sakai/rest/resource/Roster.java:31: boolean isShowWeighted = "true".equalsIgnoreCase(showWeighted); Use static final for "true" or Boolean.TRUE.toString()
Thanks for the feature... Here are my comments... plz, ignore the fact that some seem hypocritical given the current state of the codebase :) http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... File client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/view/panel/MultiGradeContentPanel.java (right): http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/view/panel/MultiGradeContentPanel.java:466: case POINTS: you can add format templates to I18nMessages.properties and create an instance of I18nMessages to get the associated methods.. add the appropriate number of params to the methid call to fill in the spots .. see line 522 of ItemTreePanel in trunk http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/view/panel/MultiGradeContentPanel.java:633: Thanks for working out a solution for this :) http://codereview.appspot.com/4172043/diff/1/client/src/java/org/sakaiproject... client/src/java/org/sakaiproject/gradebook/gwt/client/gxt/view/panel/MultiGradeContentPanel.java:1051: if (showWeightedString != null && showWeightedString.equalsIgnoreCase("true") && (!"Grade Override".equals(myCm.getHeader()))) .... I18n.getGradeOverride().equals(myCm.getHeader()))) ... and consider Boolean statics toString() values http://codereview.appspot.com/4172043/diff/1/server/src/java/org/sakaiproject... File server/src/java/org/sakaiproject/gradebook/gwt/sakai/GradebookFinalGradeSubmissionController.java (right): http://codereview.appspot.com/4172043/diff/1/server/src/java/org/sakaiproject... server/src/java/org/sakaiproject/gradebook/gwt/sakai/GradebookFinalGradeSubmissionController.java:61: not sure why the default was descending order anyways :)
