[deleted]
Ask HN: Does "trust" eliminate the need for code reviews?
51–60 of 77 posts
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#52No, of course not, everyone makes mistakes. But also the point of code review is not just to catch mistakes, suggest improvements, etc, but as a way to learn and keep up-to-date with the code base, and also to pick up tips or whatever from other developers. And I guess a meta-point is it's one of a number of practices that makes the team a team.
Making the team “a team” is the silent work that people love to ignore in our field. Software is incredibly sensitive to team composition, IMO. It’s the biggest unspoken factor in “why did this take so long?” Well, bob, because we all had to figure out what each other was doing and thinking for their pieces before we could do that for our own pieces.
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#53To everyone responding “no”: how certain are you that your colleague didn’t just skim your +5,000/-5,000 patch and slammed down an LGTM in the comments? Okay, maybe they put a few nitpicks in to make it seem like they looked closely. You still have to trust your team to actually do reviews.
I agree. But I think you've flipped the original question, which presupposes "trust." Your formulation is "does code review eliminate the need for trust?" Which is clearly a no, for the reasons you've outlined.
A 10000 LOC review isn't usually as bad as it sounds. Once you get some experience reviewing, it becomes easier to separate the critical areas from the boilerplate, and get a lot of value out of a 10min read-over. Most of the time spent on the review should be thinking about the implications of the code, not on passively reading it.
It's also helpful to think adversarially: "How can this code be broken?" This is much easier to do to someone else's code than to your own, because you haven't spent hours developing assumptions about it while writing it.
Sometimes the problem is that a patch does too many things at once. Those can be the most important to review.
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#54> merge first, for the sake of high velocity I think what constitutes high velocity or best practice is going to be highly dependent on the context. What's the risk and consequences of a mistake, for example? If you are making some Crud SaaS and you are in an early/startup setting then obviously you can probably correct a mistake pretty quickly. Perhaps you haven't even shipped the product publicly. Worst case a serv…
The OP doesn't state not to review code anymore. I think the most common process is something like:
feature branch -> review -> merge intro trunk -> release to prod
This is simplified, somewhere in between those stages is often automated test, release to staging server, feature toggles, etc. But lets omit those from consideration. Now the proposed way of working would be:
feature branch -> merge intro trunk -> review -> release to prod
You still have the same checks, you just change the order. Trunk could be broken more often, bringing velocity down. You'd have to organize review in a different way, which might be complicated. But the same checks would be there.
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#55Trust, but verify
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#56Enforcing something like this puts a lot of, IMO, undue pressure on the devs and signals that some reviewers are trying to skirt their responsibilities of being accountable for the code base.
With unreviewed code, you can more easily point fingers at someone else. Even if there's no need for that, when it is time to update the code base again, now only one person is familiar with the code, so others are less likely to take it on.
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#57No - trust informs your strategy for conducting code reviews. All code should be reviewed.
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#58Having worked on both a project that used the main-only branch strategy without pull requests, and projects that use the feature branch strategy with pull requests, I prefer the feature branch with pull request strategy.
It's easier to provide feedback and make changes prior to merging. There is usually value in the feedback that improves the quality of the merged code, whether a reviewer is requesting a change to adhere to code standards (I know, enforcement should be automated), or suggesting a different way to implement the change.
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#59Oof. Difficult one. I've dealt with multiple people and there are some I would blindly trust because I know they would do the right thing. On the other hand I've worked with people who need more reminders (eventually we had to discontinue our relationship) to do the right thing: Does the PR make sense? Did you format the code? Did the tests pass? Did you fix the tests? Or did you just comment them out? And for the pe…
Well, at least these three:
> Did you format the code?
> Did the tests pass?
> Did you fix the tests? Or did you just comment them out?
should be obvious from the MR/PR page
Re: Ask HN: Does "trust" eliminate the need for code reviews?
#60Trust, but verify.