2 ms·

> But where is the threshold where that's acceptable? Changing one line of code? One function? I think this is the key part where one simply should trust devel

> But where is the threshold where that's acceptable? Changing one line of code? One function? I think this is the key part where one simply should trust developers to do the right thing. I "self-approve" typo fixes. I might even self approve small enough code fixes. But the key is - I choose that myself and there is a treshold. It might be slightly different for each developer, and that's of course both good and bad. But regardless - the reason it has to be that way, is because the alternative is unacceptable: where the process is so rigid that every trivial change such as a typo MUST be approved by a second person. Having people draw arbitrary lines for what kind of complexity warrants a review might sound bad, but it's infinitely better than the alternative.

iainmerrick 5y ago
I sometimes self-approve small changes too. But I really do think it's a bad habit and unnecessary. I don't like hassling people with tiny trivial code reviews, but I don't mind at all when people ask me for the same, so I don't think it is actually a hassle. Catching bugs is only one of the reasons for doing code reviews (and not even a great reason at that -- plenty of bugs make it through code review after all). Another important reason is sharing knowledge within the team, and that applies to tiny changes as much as large ones. I don't think mandatory code reviews regardless of size are really "rigid" or "unacceptable" at all, not in a good team where code reviews are a well-established part of the culture. In jobs where I didn't have admin privileges and wasn't able to bypass code review even for trivial changes, we reviewed absolutely everything and got on just fine. Sure, sometimes the reviewer barely looks at the change and just rubber-stamps it. But that's a sign of trust, which is a good thing, and still gives you that little bit of team cohesion and knowledge sharing -- at least one other person is roughly aware of what you're doing.

iainmerrick 5y ago

I sometimes self-approve small changes too. But I really do think it's a bad habit and unnecessary. I don't like hassling people with tiny trivial code reviews, but I don't mind at all when people ask me for the same, so I don't think it is actually a hassle. Catching bugs is only one of the reasons for doing code reviews (and not even a great reason at that -- plenty of bugs make it through code review after all). Another important reason is sharing knowledge within the team, and that applies to tiny changes as much as large ones. I don't think mandatory code reviews regardless of size are really "rigid" or "unacceptable" at all, not in a good team where code reviews are a well-established part of the culture. In jobs where I didn't have admin privileges and wasn't able to bypass code review even for trivial changes, we reviewed absolutely everything and got on just fine. Sure, sometimes the reviewer barely looks at the change and just rubber-stamps it. But that's a sign of trust, which is a good thing, and still gives you that little bit of team cohesion and knowledge sharing -- at least one other person is roughly aware of what you're doing.