4 ms·

Exactly. I want my code peer reviewed partly because it makes it clear and formal that while we succeed as a team we also fail as a team. It’s much easier to ta

Exactly. I want my code peer reviewed partly because it makes it clear and formal that while we succeed as a team we also fail as a team. It’s much easier to talk about failures when a failure doesn’t have a single person attached to it.

mpweiher 5y ago
> I want my code peer reviewed TFA: > Engineers [..] request reviews when they think it's necessary. Problemo solved.
- pbalau 5y ago
  Good engineers will ask for code review. Most engineers won't.
  - mpweiher 5y ago
    > Good engineers will ask for code review ...when they need it. Actually, really good engineers will ask to pair when they need it. Review is a distant second.
    - btschaegg 5y ago
      > Review is a distant second. I agree with you there, but I'm also sure that one reason for this is that many people still see reviews as the task that only happens after everything else. I'm convinced there's a lot of useful middle ground to be had if reviews aren't just "here's 500 LOC, please review", but maybe more… staged. Say: Rummage around in the code a bit for yourself (ask questions if you have to), come up with a strategy of solving your problem, and submit that for review. Not in code, but maybe using markdown, preudocode, small diagrams or the like. In person (i.e. via screenshare nowadays). And if the other party doesn't find obvious holes in your strategy, then go implementing it. I remember an interview with some Linux kernel maintainer that mentioned the same effect -- people submitting a gigantic patch for something that will simply never be merged. And all that would have been necessary to avoid this wasted work was an e-mail asking if the maintainers are at all interested in the change.
- alkonaut 5y ago
  Sure. But if you read my previous message this relies on people being comfortable asking. I am but not of my colleagues are. The formal process would let them use the benefit that I can use when I think it’s necessary.
  - mpweiher 5y ago
    > people being comfortable asking. I am but not [all] of my colleagues are. Then that's what you need to fix.
    - alkonaut 5y ago
      Yes. And formal processes are 10x easier pthan magically “adding trust” or “confidence” or “seniority”.
- ahtihn 5y ago
  If code reviews aren't an expected part of the development process and there's pressure on delivering, it won't happen.
  - mpweiher 5y ago
    Fix that.
  - Ntrails 5y ago
    I don't believe that to be true. I believe the expected outcome is that people will still want code reviews on anything vaguely complex as insurance against fuckups. "Shit, WE missed something" is a nicer place to be than "I was sure I was perfect and I'm actually an idiot". It does however mean you don't sit around waiting for someone to approve a three line delete PR that you know is safe. The "Rubber Stamp" review is a thing and it does waste time.
    - btschaegg 5y ago
      Since I am currently working in an environment that resembles GP's premise, I have to say that I find this take to be way too optimistic. I'd be exactly in the boat you describe (and actually: I've yet to see a situation where someone reviewing my code did not lead to improvements). But: If I want my code reviewed, I have to actually fight for it. Since everybody's workload is too high, even the willing often simply don't have the time. From higher up, at our place, there's no one that actively opposes code review as a practice, but they don't seem to get that it requires investment (i.e. time), either. That means that if management refuses to change their ways of planning or explicitly enshrine reviews as priority tasks, it simply won't happen except for maybe the gnarliest cases. And that's from a perspective of the change's author being willing and proactive. I work with individuals that produce very questionable code at times. And I'm not talking style here: Code in 2021 that still is prone to SQL injection at every turn. Methods that are hundreds of lines long with a gigantic cyclomatic complexity that no one (including the author) will ever really understand. Method names like "process" and "process2". And so on. Needless to say, those programmers will never push for their changes to be reviewed. The rest of the team simply discovers them after they blow up (which they do regularly) and we're debugging. Now, these people are luckily a tiny minority (which at least makes this somewhat bearable), but I over and over again would wish for someone with more authority to show that they're interested in the matter. Which a policy on code review would do -- if only as a signal. Yes, we want to check each other's code. Yes, we want collaboration. Yes, that's more important than some arbitrary deadline for minor feature X. Yes, we expect our programmers to be professionals. Another symptom of such an environment also is the fact that everyone is the king of their particular hill. No one every familiarizes themselves with another person's code unless they really have to. By now, I've jumped to insisting that every change on a critical component I'll ever make will be done through pair programming, and so far, at least that seemed to stick. But again -- that only happens if both parties involved actively fight for it instead of everybody understanding that this is just an expected part of work.

mpweiher 5y ago

> I want my code peer reviewed TFA: > Engineers [..] request reviews when they think it's necessary. Problemo solved.

pbalau 5y ago
Good engineers will ask for code review. Most engineers won't.
- mpweiher 5y ago
  > Good engineers will ask for code review ...when they need it. Actually, really good engineers will ask to pair when they need it. Review is a distant second.
  - btschaegg 5y ago
    > Review is a distant second. I agree with you there, but I'm also sure that one reason for this is that many people still see reviews as the task that only happens after everything else. I'm convinced there's a lot of useful middle ground to be had if reviews aren't just "here's 500 LOC, please review", but maybe more… staged. Say: Rummage around in the code a bit for yourself (ask questions if you have to), come up with a strategy of solving your problem, and submit that for review. Not in code, but maybe using markdown, preudocode, small diagrams or the like. In person (i.e. via screenshare nowadays). And if the other party doesn't find obvious holes in your strategy, then go implementing it. I remember an interview with some Linux kernel maintainer that mentioned the same effect -- people submitting a gigantic patch for something that will simply never be merged. And all that would have been necessary to avoid this wasted work was an e-mail asking if the maintainers are at all interested in the change.
alkonaut 5y ago
Sure. But if you read my previous message this relies on people being comfortable asking. I am but not of my colleagues are. The formal process would let them use the benefit that I can use when I think it’s necessary.
- mpweiher 5y ago
  > people being comfortable asking. I am but not [all] of my colleagues are. Then that's what you need to fix.
  - alkonaut 5y ago
    Yes. And formal processes are 10x easier pthan magically “adding trust” or “confidence” or “seniority”.
ahtihn 5y ago
If code reviews aren't an expected part of the development process and there's pressure on delivering, it won't happen.
- mpweiher 5y ago
  Fix that.
- Ntrails 5y ago
  I don't believe that to be true. I believe the expected outcome is that people will still want code reviews on anything vaguely complex as insurance against fuckups. "Shit, WE missed something" is a nicer place to be than "I was sure I was perfect and I'm actually an idiot". It does however mean you don't sit around waiting for someone to approve a three line delete PR that you know is safe. The "Rubber Stamp" review is a thing and it does waste time.
  - btschaegg 5y ago
    Since I am currently working in an environment that resembles GP's premise, I have to say that I find this take to be way too optimistic. I'd be exactly in the boat you describe (and actually: I've yet to see a situation where someone reviewing my code did not lead to improvements). But: If I want my code reviewed, I have to actually fight for it. Since everybody's workload is too high, even the willing often simply don't have the time. From higher up, at our place, there's no one that actively opposes code review as a practice, but they don't seem to get that it requires investment (i.e. time), either. That means that if management refuses to change their ways of planning or explicitly enshrine reviews as priority tasks, it simply won't happen except for maybe the gnarliest cases. And that's from a perspective of the change's author being willing and proactive. I work with individuals that produce very questionable code at times. And I'm not talking style here: Code in 2021 that still is prone to SQL injection at every turn. Methods that are hundreds of lines long with a gigantic cyclomatic complexity that no one (including the author) will ever really understand. Method names like "process" and "process2". And so on. Needless to say, those programmers will never push for their changes to be reviewed. The rest of the team simply discovers them after they blow up (which they do regularly) and we're debugging. Now, these people are luckily a tiny minority (which at least makes this somewhat bearable), but I over and over again would wish for someone with more authority to show that they're interested in the matter. Which a policy on code review would do -- if only as a signal. Yes, we want to check each other's code. Yes, we want collaboration. Yes, that's more important than some arbitrary deadline for minor feature X. Yes, we expect our programmers to be professionals. Another symptom of such an environment also is the fact that everyone is the king of their particular hill. No one every familiarizes themselves with another person's code unless they really have to. By now, I've jumped to insisting that every change on a critical component I'll ever make will be done through pair programming, and so far, at least that seemed to stick. But again -- that only happens if both parties involved actively fight for it instead of everybody understanding that this is just an expected part of work.