Live data from Hacker News

Architecture vs. Implementation Code Reviews

arguingwithalgorithms.com

11–12 of 12 posts

Re: Architecture vs. Implementation Code Reviews

#11

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 sign…

Re: the first point. I agree that large changes should always be broken down into individually testable pieces, however I don't agree that they can always be broken down into individually shippable pieces. This is definitely true of large refactor projects, where a simple change (like upgrading a third party library) will unavoidably trigger thousands of changes throughout the codebase. I find it easier as a reviewer to look through a bunch of TODO comments that outline what needs to be done, and then each subsequent review addresses a manageable amount of the TODOs. I really see no other way since we can't ship two copies of, say, jQuery to customers.

I totally agree with you that the architectural changes should be agreed upon in advance, with direct conversations happening and whiteboards if necessary. I tried to make it clear in the post that architecture design should happen before any coding begins, but the gap I noticed was that even with full agreement on a schema or even down to the function level, there are still important details to work out before writing logic and UI on top of it, and that requires special attention from reviewers.

Just to play devil's advocate though, if you avoid face-to-face design sessions and do everything through code reviews, you will be forced to explain your thinking in text, and the whole conversation will remain as a record for future developers getting familiar with the code who may be confused about why things were done a particular way. The code review paper trail is immensely helpful to understanding a large codebase - reading through sparse meeting notes is never as good.

Re: Architecture vs. Implementation Code Reviews

#12
post #6

Earlier quoted context omitted.

That's quite a thorough list, which I appreciate, but on the other hand its a lot to do for an interview.

That's why I give them at least a week to do the code submission at home. It is too much to cover in just a few hours in an interview.

I have blogged about an alternative [here](1). The summary is: ask them to contribute to any open-source project of their choice. Win-win for all, IMO.

  (1) : http://lavadip.com/3948_opensource_hiring_and_lowhangingfruit.html
Post reply on HN