Code review guidelines

Posted by

General Design / Execution

  • With a recent pull, does your code build and compile?
  • Does your code meet the minimum requirements for service design?
  • Is the code understandable?
    • Properly and clearly named variables
  • Clearly defined logic
  • Is code duplication avoided?
    • Do objects already exist that you can call?
  • Does your code offer extensibility?
  • Is the function / class / instance properly sized:
    • The function isn’t trying to do too much that will be difficult to maintain.
    • How long are the methods?
  • Has the code followed proper segregation regarding?
    • Data Access Layer
    • Business Layer
    • Presentation Layer
  • Are you using framework features vs writing custom code?

Security

  • Does your code meet the minimum requirements for the security standards?
  • Data sharing with third parties is in-line with data sharing agreement
  • Application code does not expose more data elements than necessary
  • Credentials are not saved in plain text
  • OWASP guidelines are followed for publicly facing endpoints

Documentation

  • Is your code documented both header, inline and story / tasks?
    • Inline:
      • Not just what you are doing, but why you are doing it.
      • Mention any future items that need to be addressed.
    • Header:
      • Are you using the established format for documentation?
    • Story:
      • Are you updating your tasks with current status?
      • Have you conveyed your logic into your story?
      • Have you updated the story so that QA and the business owner will understand your functions?
    • Are system diagrams updated to account for your application flow?

Testing

  • Can your code be tested / unit tested easily?
    • Have you tested your code?
  • Have you run through the acceptance testing that QA is going to run it through?

Error Handling

  • Error handling on all modified code / new code
    • Bubble errors up to main thread
    • Log errors to DB for debug later
  • Have you tried to break your code? Pass in incorrect values to trigger the error handling?
  • Are you logging the error to a central location?
  • Is production support aware of the error handling you have built?
  • Ensure that you are not logging sensitive information in a log table.

 

Leave a Reply