Architecture vs. Implementation Code Reviews

  • I don't like the idea having long-running branches and especially TODO comments in code before a review. I would argue that it is better to discuss large architectural changes as a team, and split the task into smaller, more manageable tickets and branches which can be reviewed and/or QA'd individually. I think this is key to maintaining clean and reliable.

    I think the approach outlined is a bit upside down, and significant API changes should be agreed on first. Not necessarily documented or thought about in detail, but the implementer should walk away from the preliminary discussion with a good idea of the agreed structure in order to be able to flesh it out further.

    This means the architectural changes on a high level are understood and agreed on by the some or all of team first and the implementation is what actually needs to be reviewed afterwards.

    Thinking about a solution, pushing a draft to code review with your thoughts explaining the changes typed out in full, responding to comments on the review two days later, and repeating the process all over again- it just seems like if you need this sort of feedback it would be way more efficient to schedule an informal exchange in front of a whiteboard with one or more team members.

  • Very nice article. To be honest, I always review code from a architectural perspective, rather than just reviewing code quality, as getting that stuff wrong will hurt a project more so.