2010-05-10 Code Review
Call-in Information
Call+1-712-432-0075* or Skype a voice call to freeconferencing
Enter the code*305801
Screen sharing
Attendance
Jeremy Keiper
Sy Haas
Win Ribeka
Wyclif Luyima
Ben Wolfe
Tickets to review
unnamed link display creator
unnamed link encounter search
unnamed link boolean person attribute types
unnamed link form auditing
Ticket 1985
Approved
Ticket 2001
EncounterDAO needs CTRL+SHIFT+O to organize imports (bad import: org.openmrs.)
PatientSearchCriteria needs CTRL+SHIFT+O
methods need @returns in javadocs
removePadding method, replaceFirst with an empty string??
line 207 should be replacing 2 spaces with 1 space, not empty string
line 237-238 need formatting
HibernatePatientDAO needs CTRL+SHIFT+O
EncounterServiceImpl needs CTRL+SHIFT+O
fix formatting
HibernateEncounterDAO needs CTRL+SHIFT+O
DWREncounterService, line 30 should be private static final
line 33 remove the 'h1. true'
EncounterServicetTest needs CTRL+SHIFT+O
EncounterService needs CTRL+SHIFT+O
the @should's should match the names of the test methods (http://openmrs.org/wiki/Generate_Test_Case_Eclipse_Plugin)
add in @since 1.7 to new methods Ticket 1955 h1. Approved Ticket 260 ==
AuditFormFieldController
line 55 should be @see, and move @Override to right above method name (not in javadoc)
line 66 and 67 should be moved into the 'try' and 'catch' block respectively to remove the local variables, along with lines 83-86 to set the session attributes accordingly
line 73 and 78 should be internationalized
remove method formBackingObject
openmrs-servlet.xml: remove commandName
FormService
line 226 remove @param because it does not exist in the method signature
line 224 expand on javadoc comment
auditFormFields.jsp
internationalize messages in messages.properties
FormServiceImpl
remove fieldIdAsKeyAndFieldNameAsValueMap and use the fieldNameToIdMap on 723
use OpenmrsUtil.nullSafeEquals for fieldsAreSimilar method
use Concept.equals instead of Concept names
remove comment on fieldsToDelete (737) and remove the dao.deleteField (line 738)
FormServiceTest
line 470 is not needed because of INITIAL_CONCEPTS_XML is the same
make line 468 a constant