Sure, that’s the theory. In practice code review often looks like this:
- a quick glance to see if the code plausibly does what it claims for longer patches
- A long argument about some stylistic choice for short patches
In other words – people were barely reading merge requests before. Code reviews have limited effects as well. You won’t catch all bugs or see if it actually works just by looking at the code. Code reviews mainly serve to spread knowledge about the code among the team. The more code exists in a project, the harder it is to understand. You don’t want huge areas of code, that only one person has ever seen.
heluecht@pirati.ca 2 weeks ago
Well, on Friday I spent around 1.5 hours just reviewing a single PR. And I'm not done. I will have to continue my work on it on Monday. Reviewing in our company means understanding the connected use case, then having a look if the coding does what the use case defined. Also we look if the coding is done according to our internal style guide. Since our review is normally done by at least two people, (at most of our apps two people have to accept the PR until it can be merged) one person will see what the other missed. And we often talk about what the other missed, so that we learn.
Concerning angry customers: Our apps are used by several then thousand users. And although our group doesn't have direct customer contact, we get the bug reports and have to fix them anyway or we have to support the teams who directly work with the customers.
And I just realize that I'm in a very lucky situation. In our company echt use case is tested thoroughly by the responsible QA and PO. And for each use case we write half a dozen (or more) test functions that check the functionality. Normally coding the tests takes more time then coding the use case itself.
Our company is very AI driven, but on the same hand we hear in the regular town halls about the customer satisfaction. And the goal there is to increase it steadily. Our customers are companies, so maybe there's the difference.
TehPers@beehaw.org 2 weeks ago
To put some perspective into what our code looks like, there are very few tests (which may or may not pass), no formatter or linter for most of the code, no pipelines to block PRs, no gates whatsoever on PRs, and the code is somewhat typed sometimes (the Python, anyway). Our infrastructure was created ad-hoc, it’s not reproducible, there’s only one environment shared between dev and prod, etc.
I’ve been in multiple meetings with coworkers and my manager talking about how it is embarassing that this is what we’re shipping. For context, I haven’t been on this project for very long, but multiple projects we’re working on are like this.
Two years ago, this would have been unacceptable. Our team has worked on and shipped products used by millions of people. Today the management is just chasing the hype, and we can barely get one customer to stay with us.
The issue lies with the priorities from the top down. They want new stuff. They don’t care if it works, how maintainable it is, or even what the cost is. All they care about is “AI this” and “look at our velocity” and so on. Nobody cares if they’re shipping something that works, or even shipping the right thing.
heluecht@pirati.ca 2 weeks ago