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

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