51–60 of 315 posts
"Trust, but verify"
I think a lot of other commenters nailed the trust equation in more verbose terms. I would add that in some industries or for certain customers, a 2+ person code review rule is mandatory for compliance, regardless of any perceptions around how mundane the changeset might be.
Absolutely not. This is a recipe for disaster and a terrible example to set for any upcoming engineers that haven’t had much industry experience yet. Code reviews help me be more confident in my own work. Even if 90% of the time it’s fine, there’s always that 10% where a second pair of eyes catches a mistake or offers a suggestion that makes the code even better.
Meh. If 90% of the time it's a mindless ritual there is probably a better way to achieve the same goal. Code reviews are a huge time sink, especially for unimportant style- and naming nits. Code review as mentoring can be great, if it's a directed 1:1 effort. Usually, it is not.
Earlier quoted context omitted.
If something hit production and caused a major fuck-up because there was no peer review process, then in all the places I've worked at the first action item in the post-mortem would be "we should introduce peer review." Otherwise someone would ask how we could ensure it wouldn't happen again, and no one would be able to say "because we trust them," and leave it at that. It would sound more like "I trust you will neve…
This post sounds like wisdom, but after looking at it hard, it seems that the point is no more than "peer review processes are popular". Not to say that code review is bad in any way (I do it at my current role), but, this argument for them is not very good. One downside of code review is reduced velocity. There are teams out there who use code review so effectively that they never ship big fuck-ups to production. Th…
I can see how that equation balances in favour of no code review when it's in the context of a startup that is probably still figuring out how to make money, especially with a subscription based app launcher. As far as prototyping and early stage iterations go, you're probably just seeing what works and reviewing the code isn't so important at that point. There would still be one or two things you'd ask someone to check, e.g. security related stuff.
I think you make a good point though: what makes an effective code review process? I don't think there's a single answer to that because it depends on the circumstances.
I would at least say that it goes better if it's treated as a priority and issues are raised and dealt with promptly, rather than leaving it as a chore you eventually get around to. And you would have to see value in it beyond it being a sanity check, as others in this thread have described (knowledge transfer, for example).
Code reviews make sure the other party at least thinks a bit about the code they write. "Someone is going to look at this, let's at least tidy it up or not be cobbled together underperforming spaghetti". Sure, you can trust your co-workers but do you trust them enough that they have that thought every day of the year?
Earlier quoted context omitted.
The best code I have seen was on projects without code review. And projects with it were more mess - they were surface consistent but overall hard to comprehend. The deciding factor was ownership and accountability tho - you maintained own code and if you done it crappily, you knew. The code review is related to assumption that everyone can change everything - meaning all in all inconsistent mess. It is also related…
I agree that code review is not a great medium to disseminate information about the codebase, but assigning individual ownership and accountability doesn't make the need to share information go away. Ownership and accountability are workable as long as the same person sticks around long enough to fix all their mistakes. If that person leaves, the code becomes orphaned and it's up to one of their teammates to find out…
And the person doing explanation actually understand the whole part. That is big one too - you are not explained bits of it by someone who knows only small parts of it. And the system was not changed by third party without the person who explains having at least vague idea it was changed.
> Ownership and accountability are workable as long as the same person sticks around long enough to fix all their mistakes. If that person leaves, the code becomes orphaned and it's up to one of their teammates to find out where the skeletons are buried.
That is actually exactly the same without clear areas. Except everyone is new all the time.
If you have high turnover, this aspect will be bad matter what.
And it is not that bad with responsibility either. Old person does explains things to new person. Taking over, when you are working consistently in same area is not that hard, actually. Some parts you conclude to be mess, but usually not whole of it.
---------
And imo, quality and knowledge heavily depends on other parts of team. Analysis and testing. Both, if work reasonably well are massive resource for knowing how the hell the system is supposed to work.
And once you have reliable source for requirements, that is not developers, then deciphering the code is much easier.
> Pull requests don't prevent bugs. Is this some anti-vax satire? Of course pull requests (code reviews) reduce bugs. And of course some slip through. It doesn't need to be 100% to be useful.
I haven't seen any research that supports either. I wouldn't be surprised to learn that code review does nothing other than share knowledge, or to learn that it reduces bugs by 50%. I just honestly have no idea, I do code reviews, because we do code reviews. In fact I have seen alarmingly little research about code management. Code review, standup, agile, etc. does any of it do anything useful? I only came across ane…
I think it is unquestionable that code-reviews catch bugs. Its value as a use of time is another question.
Absolutely not. This is a recipe for disaster and a terrible example to set for any upcoming engineers that haven’t had much industry experience yet. Code reviews help me be more confident in my own work. Even if 90% of the time it’s fine, there’s always that 10% where a second pair of eyes catches a mistake or offers a suggestion that makes the code even better.
Meh. If 90% of the time it's a mindless ritual there is probably a better way to achieve the same goal. Code reviews are a huge time sink, especially for unimportant style- and naming nits. Code review as mentoring can be great, if it's a directed 1:1 effort. Usually, it is not.
This isn't a problem with code reviews, this is a problem with your team's processes that code review has highlighted. These problems will show up in others ways if not during code reviews since clearly there's strong disagreement on coding styles and conventions. So go and fix or implement the style guides, linters and so on.
This is like saying that the way to fix fevers is to get rid of thermometers.
I'm glad this works better for them, or at least they feel that it does. I'd also bet that they're either a small minority of teams that jell so well that they manage to scale a codebase without it turning into a disgustingly inconsistent mess; or that indeed their codebase qualifies as such or is still too small for the effects to be very visible. Code reviews are not a tool intended mainly to catch bugs. Types, aut…
Earlier quoted context omitted.
For many developers, people merge their branches directly to production so the pull request review is where that checking happens.
Ok, but many companies don't use such ridiculous process.