Stop Doing Code Reviews

Recorded live on twitch, GET IN

Become a backend engineer. Its my favorite site

This is also the best way to support me is to support yourself becoming a better backend engineer.

MY MAIN YT CHANNEL: Has well edited engineering videos

Discord

Hey I am sponsored by Turso, an edge database. I think they are pretty neet. Give them a try for free and if you want you can get a decent amount off (the free tier is the best (better than planetscale or any other))

[spamviking]

The author wouldn't say this if you'd seen some of the code that I've seen juniors make a PR for.

[Mastikator]

After having worked with trunk based commits and peer programming, I have to say that code reviews should be done in-person/via voip. TALK TO EACH OTHER. Literally discuss the code while looking at the code. Communication is SO UNDERRATED in the programming world.

[Seedzification]

Code reviews are essential when dealing with juniors or newcomers into a code base..

[TheVideogamemaster]

The company I worked for liked to hire people who couldn't actually code (or speak English) just so they could have a "diverse" workplace, so I was constantly tasked with stopping their service-breaking code from making it to production. If I wasn't reviewing their commits, the company would not currently exist.

[sadboisibit]

Coworker: This function seems too complicated for me.
Me: Skill issue.
*merges anyway*

[lpprogrammingllc]

Advocating pair programming instead as an absolute is silly. Some problems are NP-complete. They might involve hours, or even days of programmer time, but adding a second programmer at best reduces the wall-time by minutes (and at worst increases it by hours). Once the solution is written, verifying it works is _much_ easier than learning all the dead-ends that were explored and discarded along the way.

This most often comes up when trig or calculus shows up in a non-trivial way. Having someone along for the ride while I'm working on a whiteboard figuring out which trig identities to apply to find some value either has the second programmer sitting idle, or asking questions and grinding progress to a halt. But once the code is written (complete with comments explaining which identities are applied), that second programmer can verify the chosen identities are correctly applied quite quickly (and can verify they _are_ the correct identities if required).

[DamnedVik]

> For instance, sharing about coding guidelines, architecture, coding practices

At my job we have a code style that the whole department should follow, we ask developers to install IDE plugin that will format code automatically, we ask developers to install git-hook that will format code before commit, we added a build-plugin that will fail compilation if the code doesn't follow the code style. And people still didn't follow it.

Many people over-complicate stuff, they write code that doesn't make sense, they can make a simple for loop make N^2 number of iterations.
Even "senior" developers that apparently have 7-11 years of experience do all of this.

And I'm not even talking about devs who implement 3-5 features in a single PR and then expect others to review all of this in a couple days

[stubbqaz]

We tend to forget that web development with a 2-year code lifecycle till obsoletion is not all software. Code reviews look differently and have different purpose if you develop software for heavy industries, cars, aircrafts - you design the code for a decade ahead. Sustaining healthy architecture and avoiding hack-arounds is a very important objective for those platforms and it is often painful for developers to conform and develop tedious and substandard solutions in the name of preserving the idea that the architect had.

[seancooper]

If you're doing pair programming you still need someone else to do the code review.

[ddomingo]

I worked at a French startup called Sunday that did trunk based development and pair or mob programming all the time. It was painful but boy did we fly. The speed at which we added changes was nothing like I had ever seen. The difficulty though was finding the right person to pair with and in my case the timezone difference but it was extremely effective. Stressful, but fast.

[nemesis-ad]

Problem is not Code Reviews, problem is when developers create a mega pull request with 20 files changed. Instead of changing the way the code is reviewed, we should change the way MRs are created. Splitting a Mega MR into smaller ones is always a great way and almost always possible.

[WiseWeeabo]

So, basically we should stop doing code reviews because this one guy saw some bad reviews?
Just go and have a conversation with these people who are leaving cryptic and useless comments on PRs..

[Veptis]

As a very junior developer. I started writing my first PRs a few months ago. And I really enjoy the idea of having the affirmations of a code review.

I got to excited and opened a nit PR on other repos (like adding hyperlinks that were missing in the readme). And got shot down. Which demotivated me there.

Now I am asked to maintain the feature myself as its own repository and still having a reviewer approve it is great. I also like how clean the repository is if all commits a PR merges.

[EmperorRobin]

Often the real problem is a social one.

If you work with people who hate their job/project then they either:
procastinate with excessive code reviews in order to not have to do their job
or they see it as part of their job and do nothing/ghost or tldr.
Making code reviews a bad thing.

On the other hand if people like their job/are passionate, then you get great feedback and code reviews are good.

[rainerwahnsinn]

That article is crazy talk to me. Code Reviews on Pull Requests is the most important thing imho. Of course, for larger changes, talk about it first - but still, a PR is the place to talk about issues that come up with the code that is coming in, but also with the code that is already there. At this point, you have Code. Code it fact. No fancy architecture diagrams to hide behind.

[wpelfeta]

I feel like most of these could be solved by simply establishing a few guidelines for reviewers, like spending X hours a day on reviews, assigning reviewers, allowing reviewers to remove themself if they are too busy, letting merges through and creating a separate tech debt to address more complicated issues, etc. At least, I don't really have most of these problems.

Honestly sounds like a junior dev who got dinged too hard a few too many times for his code.

[cfehunter]

No matter what experience level you're at code reviews are absolutely essential.
They help spread information about what's going into the code base and they're a useful learning tool.

If it's a bottleneck, then do them post commit, but you should still do them.

[silak]

I've done code reviews on a project which doesn't have linters and formatters set up, where a more senior developer still just let his editor go crazy with the auto formatting... It was pure hell to review...
I do however honestly think that most bad things about code reviews happens because of bad practices. E.g. doing many things in the same PR, leaving PR's hanging, and so on.

[joehead]

if you aren't having an explicit set of meetings to decide:

1. enforcing code styles via linters & formatters
2. code review comment thread ping pong limit (1 allotted back and forth then get on a call and resolve the thread with a final comment)
3. r.o.e. for tagged reviewers (dm reviewers directly when pull request is created and then again after 24hr of inactivity)
4. removing subjective nonsense from reviews (nit picks). track and put these topics in the next meeting and add to step 1
5. naming conventions (code/branch)

then what is your team even doing?

[thekwoka]

The articles issue with the "redesign review" was pretty bad...

It's demoralizing? What the heck does that matter? If the way they went about solving the thing or making the feature has major design issues, either from lack of knowledge of the stuff, over engineering, or whatever.