onion2kMar 17

But you can’t just not review things!

Actually you can. If you shift the reviews far to the left, and call them code design sessions instead, and you raise problems on dailys, and you pair programme through the gnarly bits, then 90% of what people think a review should find goes away. The expectation that you'll discover bugs and architecture and design problems doesn't exist if you've already agreed with the team what you're going to build. The remain 10% of things like var naming, whitespace, and patterns can be checked with a linter instead of a person. If you can get the team to that level you can stop doing code reviews.

You also need to build a team that you can trust to write the code you agreed you'd write, but if your reviews are there to check someone has done their job well enough then you have bigger problems.

Show threadanal_reactorMar 17

I never review PRs, I always rubber-stamp them, unless they come from a certified idiot:

1. I don't care because the company at large fails to value quality engineering.

2. 90% of PR comments are arguments about variable names.

3. The other 10% are mistakes that have very limited blast radius.

It's just that, unless my coworker is a complete moron, then most likely whatever they came up with is at least in acceptable state, in which case there's no point delaying the project.

Regarding knowledge share, it's complete fiction. Unless you actually make changes to some code, there's zero chance you'll understand how it works.

Show threadworldsayshi
People always makes mistakes. Like forgetting to include a change. The point of PRs for me is to try to weed out costly mistakes. Automated tests should hopefully catch most of them though.
Mar 17 at 7:16amWeb
Show threadFargrenMar 17
The point of PRs is not to avoid mistakes (though sometimes this can happen). Automated tests are the tool to weed out those kinds of mistakes. The point of PRs is to spread knowledge. I try to read every PR, even if it's already approved, so I'm aware of what changes there are in code I'm going to own. They are the RSS feed of the codebase.