Code review guidelines
Code review guidelines
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?
- Inline:
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.