Code Review

Code Review are a great tool and you should do it often. Best Practices for Code Review gives a great summary including benefit and top 10 tips. Checklist are great tool, NASA’s Space Shuttle Operational Flight Rules is a great example. Current checklist is heavily inspired from this Checklist. Python specific elements are added to it which are inspired from Pycon 2016 Talk. Go ahead and use this checklist for:

  • Guide while starting new project

  • Refactoring an existing project

  • Rewriting a legacy project

Structure

  • Is the code PEP-8 compliant?

  • Are there any uncalled or unneeded procedures or unreachable code?

  • Can a code be replaced by standard library calls?

  • Are there any blocks of codes that be made into single function?

  • Are there any Magic Numbers or Constants?

  • Does the code have any logical organization or its all flat?

  • Are there any complex modules that can be broken down into smaller chunks?

Documentation

  • Is there any documentation?

  • Does the code explain what main module and entities are?

  • Does the code have any comments?

  • Are comments in these code meaningful?

  • Do we have key classes/functions documented using right? {Read more on PEP257 for more details}

  • Do we have a README.md?

  • Do we have a WORKLOG.md?

  • Do we have a TESTING.md?

  • Are steps for setting up the project documented in README.md?

  • Do we have a documentation of Key Models in README.md?

  • Do we have documentation on key management commands?

  • Do we have documentation of Key Roles in TESTING.md?

  • Do we have a Permission Matrix?

  • Do we have a list of Critical Workflows for application in TESTING.md?

  • Do we have list of Browsers with specific versions mentioned that we will be testing in TESTING.md?

  • Do we have list of Mobile devices on which we will be doing testing in TESTING.md?

Variables

  • Does the code have abstract variables names as i,`j` and k?

  • Does the code have clear and explicit names?

  • Are there any redundant or unused variables?

Arithmetic Operations

  • Does the code have any blocks that are heavy in computations?

  • Does code avoid floating point computations for comparisions?

  • Are computation calculations taken care of divide by zero errors?

  • Are there unit-tests for computation logic?

Loops, Branches, Function and Classes

  • Are all loops and branches properly nested?

  • Are most common cases in IF-ELSEIF block tested first?

  • Are all cases covered in IF-ELSEIF-ELSE blocks?

  • Are loops terminating on valid conditions?

  • Can any statement that is within the loop placed outside the loop?

  • Can blocks of IF-ELSE statement be named into function?

  • Can blocks of functions be named into a class?

  • Do we have a Class with just one single public function?

  • Can blocks of function be moved into a module/external file?

  • Does the Function/Class follow Single Responsibility Principle?

Defensive programming

  • Do we have any blocks of code that should have TRY-EXCEPT blocks but don’t have them?

  • Are there any errors that are silent?

  • Do we have test case or unit tests written?

Django Specific Checks

  • Do we have segregated settings {local.py, dev.py, prodction.py and base.py}?

  • Do we have updated requirements.txt?

  • Do we use PostgresSQL as default DB?

  • Do we have one-off scripts that can be extracted into a Django Management Command?

  • Do we have commented urls in urls.py?

  • Are we using the right patterns for URLs? {List of Useful URL Patterns is a great reference}

  • Are we importing specific Classes/Functions from modules from accounts_manager.models import AccountsManager as opposed to from accounts_manager.models import *?

  • Do we have singular application and model names? {E.g. accounts_manager as opposed to accountsmanager}

  • Are we storing contents of email outside code? {E.g. store contents of emails in templates/emails/}. Use render-string for substitions

  • Do we have Skinny Views and Fat Models? {Reference explains with detail}

  • Are we using standard packages like django-haystack as opposed to writing custom low level queries to search indexer?

  • Do we have following integrations from day one?

  • Are we profiling code using django-debug-toolbar, do know how many queries are getting fired on most pages?

  • Are we using redis for session backend? {Reference Blogpost}

  • Do we have simple template tags? {Ideally no DB calls in template tags}

  • Are we minifing all our static assets using django-compressor?

  • Do we have our static on CDN?

  • Do we have deployment scripted using Ansible?

Samples

Following are some sample of code reviews that have been conducted

  1. Time tracking software

  2. iOS App

  3. Android App