Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

My larger point is that a code review can only happen after the programmer has changed some code. For anything that is flagged during the review it’s fair to ask: Was this requirement known before the programmer started writing code, and if so, why was it not communicated earlier?

Again, this isn’t the “fault” of the code review, it’s that there are insufficient or missing pieces of the software development process overall. Yes, having a code review is a good thing, but we often try to make it do all the things.



This is why I think architectural discussions should happen first (and typically decisions should be made collectively rather than individually). At one of the best places I worked, we discussed every new feature as a team (four engineers and a PO) first, drawing stuff on whiteboards and figuring out if we had enough information to solve the problem first. When the solution was clear enough for whoever was implementing it, and we all agreed the rough plan made sense, then that person would do away and get that done.

One of the big things this achieved was that we rarely had to rewrite stuff, or come up with a solution that was missing some vital detail. Because everyone was involved in the initial plan (UI, backend, and business), we had a good grasp of different people's needs and could challenge ideas that would cause problems in some niche use-case.

To me, implementation and code review both come after this architectural discussion. That's not to say that there aren't still decisions to make, and sometimes I'd still sit down with the other UI guy and discuss how we might best structure our code - but again (ideally) before and during implementation, rather than after it when the branch is ready to be merged.

Rather, I see code review as a chance to learn about the minor details of implementation. For example, a function to search through a list of objects might have a dozen different potential signatures, invariants, and usages, depending on who implements it and how they prefer to work. Having a review makes sure that both (or all) developers know how to use this function, and have seen it if they need to use it somewhere else in the codebase.

And sometimes none of this works out, you think you've planned everything, you write your function to search through objects, and your colleague points out that a standard library function can do everything you've written but simpler and faster. And then you'll still have to rewrite your code, but now at least you've learned something new.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: