Another—very old—rationale:
replyPeople write code differently when they know that it will be reviewed by people who will not only comment on it, but also form long-term impressions of the submitter's competence and fit based on the code that is reviewed.
I've always felt that this is and advantage to open source software. The vast majority of open source software that I've bothered to look at the code for used best practices, was reasonably secure, and was above all maintainable. The bespoke projects that I've worked on at various companies? Complete spaghetti messes almost all of them.
replyPretty much. I use OpenBSD and the basic stance is that you need to look at the code of the system and the various software in ports. Because the only way to get timely support is you helping yourself and then the community will help you. And if you find some hackish code, there’s generally a good reason it’s that way.
replyThis is the big one IMHO. I've been part of teams where there was zero code review, and where the code reviews were intense, and for sure people write much better code (generally speaking) when they know it will be reviewed. We all do it, even if subconsciously.
replyI do advocate a balance though. Ridiculous code reviews tend to slow the process down immensely, which is good for some things but bad for others. Finding a good balance is super important IMHO
OP is Short-form false-dichotomy. Thank you for this perfect response.
reply100% agree. It's as if you read the first sentence out of my mind. Thanks.
replySimilar to #3:
reply- having someone more experienced in that subdomain catch problematic parts and inconsistencies with existing code.
I agree with all of this, I read into the "single purpose" of understanding the code and complaining about what you don't understand implying that if you understand it, you will be able to point out and comment on things that are wrong /foolish/unsafe/etc after understanding it. From that perspective on the OP, it makes sense to me. Particularly with regard to modularity and factoring; once I understand all of a gigantic PR I will have modeled it in my mind and will begin to either see that it will be maintainable, or will be a total nightmare one day... or somewhere in between.
replyAfter a re-read, I realized the claim wasn't "single purpose", it was "primary purpose", in which case this makes even more sense to me. I guess everything else comes from understanding what's in a given PR. It is difficult to find bugs in code that you don't understand, and it's difficult to understand code that doesn't follow convention, etc. I think I've worked this way and just not thought about it from this perspective. I review a lot of code, and what I generally do is fire up my editor in the relevant repo and follow along. If there's a method call to outside of the PR, depending on what it is and what I know about it, I'll pull that up in my editor and review there to make sure I understand what's happening. That understanding is where the comments come from. Maybe "I understand this and it's right" or "I understand this, and it seems wrong because [something]" or "I do not understand this because [whatever]" etc. Maybe "primary purpose" isn't perfect.. perhaps "overarching goal" or similar.. :shipit:
replyI agree it doesn’t have a single purpose but you didn’t refute the claim about a primary purpose. One purpose to rule them all.
reply