Confessions of a programmer: I hate code review (2010)
Rant...<p>Was nominal lead on a smallish project a couple months back, with 2 other folks.  Worked with one before, one I hadn&#x27;t worked with before.<p>We&#x27;re all remote, and the &#x27;new&#x27; guy started doing &#x27;code review&#x27; on code before he had a running environment.  We&#x27;d taken over another codebase, and he was pretty up-front about wanting to adopt&#x2F;enforce certain stylistic standards, which... to me, I don&#x27;t particularly care about as much (on the PHP side of things, PSR-2, in this case).<p>We&#x27;re all remote, so we had a call - a couple actually - and I indicated I didn&#x27;t care all that much, but he was free to make changes as he saw fit, as long as 1) he got a working environment up and running, 2) he was also spending time writing some tests around the code as he explored it, to have a firmer understanding of what was going on, and 3) he didn&#x27;t break any existing tests (or new ones that were developed).<p>I swear that I don&#x27;t think he&#x27;d actually had a working copy of the project, but was still committing code.  Not a lot, but ... it broke tests.  It broke stuff that was not &#x27;tested&#x27; in an automated way yet, but was testable by running the app, and I think he&#x27;d just not bothered to test it at all.<p>But... hey - look at the formatting!  Look - spaces, not tabs! - how useful when someone else is having to go back and debug your crap.  I received a couple small lectures on variable naming and the importance of descriptive variable names.<p>function getListOfUsersForCompany()
{
  $res = db::query(&#x27;whatever query&#x27;);
  return $res;
}<p>Apparently, that $res is confusing and &#x27;bad practice&#x27; because it&#x27;s &quot;hard to reason&quot; about what&#x27;s going on there.  Literally 2 line methods with an internal placeholder variable (which also had some working functional tests around them) - those were being criticized by someone who had committed test-breaking code multiple times.<p>In all this, I kept having to figure out how much of my reaction was because <i>my</i> code was being criticized, vs what the criticisms were.  I got accused of taking things &#x27;personally&#x27;, but I kept coming back to &#x27;working tests broke with these changes&#x27;, and in my mind, that this code was considered somehow &#x27;better&#x27; because it more closely followed a stylistic &#x27;standard&#x27; was bothersome.  Working tests should a standard we strive for too, right?<p>I <i>have</i> worked with some folks who had a real eye for analysis - remote or colo pairing on problems has helped reason out issues that I know I&#x27;ve missed.  And when I&#x27;m pairing or doing a requested review, cosmetic style tends to be the last thing I&#x27;ll bring up, because it generally has the least impact on something.  Much prefer to have someone spend time writing tests and&#x2F;or docs and&#x2F;or insightful comments vs reformatting.  When I&#x27;ve refactored with someone to make something more testable, better naming&#x2F;comments&#x2F;docs tends to flow out of that refactoring anyway.<p>While I understand formatting is often not that big of burden, especially with modern IDEs, it&#x27;s also not terribly high value on many projects.  I&#x27;m primarily speaking on projects I&#x27;ve come in to where there&#x27;s already extant code - in this case, we had taken on a very weird hybrid of 2 PHP frameworks and 2 JS frameworks (some screens used raw vue, some compiled vue, and some compiled angular).