Code Reviews

  • Is it designed well?
    • Does it belong where it’s being added? (Is it better added to a library or a different app?)
    • Is it consistent with the rest of the program it’s being added to
    • What are its collaborators? What does the messaging with them look like?
  • Does it work?
    • Is UI work, or parallelism involved? Look harder! Try to setup a demo or ask for the work to be demoed
      • For parallel code, think about race conditions or chances of deadlock
  • Is it as obvious, simple as it can be?
    • Over-engineering: trying to make code more generic than it needs to be right now
    • Is every line understandable / not overly complex?
      • Function?
      • Class?
      • Package?
      • Too complex == can’t easily be understood by readers
  • Show me the tests
    • Units, functionals or end-to-ends should be present, and working
  • Style
    • Naming
    • Comments
    • Adherence to style guide
  • Do we need a wiki update?
  • Point out the good stuff!
  • Improve overall code health when you can




  • Hexagonal Architecture: A Netflix blog post talking about organizing a program into layers, and responsibilities in the hexagonal way. The principle objects they call out are business entities, repositories, services, data sources, and transports. Seems to me like you can go a long way with just these concepts and come to a well factored program.

hexagonal architecture


  • entities
  • repositories
  • services
  • datasources
  • transports


  • DevTernity 2017: Ian Cooper - TDD, Where Did It All Go Wrong: Excellent tdd talk

  • Write tests to requirements. Make sure the feature you’re building for a customer does what it needs to do. Test the value! This could mean integration, and end to end testing. Units are great but there’s more to think about

  • Software Engineering at Google: Lots to think about in chapter 12. On my mind right now is the idea of a different sort of classification of tests as small, medium, and large. Small tests are less brittle and fast - they don’t leave the test process, mediums are slower and fail sometimes - they can call out to another process but don’t leave a machine, and larges are expensive to maintain and flakey in general - they involve multiple processes between separate machine hosts sending messages across an unreliable network

software engineering at google book