Thread

1. dimbletimbers · 1790559269 · focus · HN ↗

   A defense of human code review I wish I saw more often, especially in light of the concerns people have about cognitive/comprehension debt: comprehension redundancy. At the end, if taken seriously, at least two people understand how the feature works (even if that number is, on average, trending closer to between one and zero). Ideally at least one of the two also comes away with a better understanding of the wider system and how the feature fits into or stands out from that landscape.

   1. atomicnumber3 · 1790560581 · focus · HN ↗

      If only one person knows the code, then PR time isn't going to save you.

      I'm tech lead and I basically don't review PRs, and I tell people this, with a caveat - if you can tell me what you specifically want me to review, for what specific purpose, I'm happy to!

      So "can you review this bit for race conditions" is great, love it. This forces people to actually think about what in their code they should be suspicious of, if anything.

      "Can you review this" [link to PR] is getting a rubber stamp because humans have never been good enough at "just spotting bugs" to make this worth it and now any LLM is better than a human.

      And for the purpose of understanding - PR time is too late. I have not reviewed a PR ("for real") in a long time and yet I could tell you how every system my people have built works down to a very fine level of detail. And it's because _we talk to each other!_ We don't just chill in the same slack channel and code independently, we all value each others brains and want each others inputs because we know it will improve our product and we value what perspectives others will bring.

      Trying to learn via PR is a sad substitute for real collaboration and teamwork.

      1. setr · 1790561929 · focus · HN ↗

         I consider PRs to be primarily a defense of the architecture, and to a lesser degree a general sanity check. It’s also a useful opportunity to enforce automations are being run

         1. atomicnumber3 · 1790562582 · focus · HN ↗

            I consider 99% of my "defense of the architecture" strategy to be teaching my team why the architecture is important, how to think about it, and invite they commentary on it as we own and evolve it together. And of course if they are doing something and want input or are uncertain, then my door is open.

            If PRs are a notable part of my architecture defense, I'm going to work on investing in the team instead of reviewing PRs.

            1. setr · 1790583529 · focus · HN ↗

               I mean, do that too, but end of the day PRs are your final chance to catch the mistakes before they start cementing, and is your best opportunity to identify misunderstandings (you can smell the confusion in their changes and address it directly).

               But also, I must defend against the hordes of unwashed masses and maintain the sanctity of my domain. End of the day, a codebase I own is a codebase I own, and others cannot be allowed to poison the well, intentionally or not. That’s how you get cholera

               1. Shacklz · 1790601640 · focus · HN ↗

                  Very well put. A PR is the final (and ultimate) chance to say "no" to a change - after that it's part of the software, it needs to be maintained, it can have impact on stability/quality etc.

                  I don't quite understand how someone in charge of a software (techlead or similar title) can't take this serious; I know a few such people and I generally prefer to avoid working with them...

Unofficial Hacker News client; not affiliated with Y Combinator.