Hacker News Reader

Comment by vinnymac | Hacker News Reader

For any PR above a few line change, if a developer has not done a self review, I don’t review it all.

Instead I request that it is self reviewed with context added, prior to requesting re-review.

I also tend to ask the question, “are any of these insights worth adding as comments directly to the code?”

9/10 the context they wrote down should be well thought out comments in the code itself. People are incredibly lazy sometimes, even unintentionally so. We need better lint tools to help the monkey brain perform better. I wish git platforms offered more UX friendly ways to force this kind of compliance. You can kind of fake it with CI/CD but that’s not good enough imo.

View on HN

By self review, you mean that the developer adds comments in the code review tool? that is a great idea, I want to try this.

I've been doing this as part of my workflow for a few years now. My coworkers have expressed appreciation around that effort.

A nice side effect is that going through a self review and adding comments to the PR has helped me catch innumerable things that my coworkers never had to call me on.

Do you see any added value in adding the comments? I just fix the original commits and force push (each dev owns their branches at $JOB), but I'm wondering if I'm missing something?

The value I get is that it helps me catch errors before I ask others to check my work, and I use my PR comments as a teaching tool (I'm one of the seniors on the team).

I thought everyone did this. I review twice. For each commit with -v and finally in GH/GL after I open the PR/MR. I often catch something on that last one.

It's rubber ducking.

What is `-v` mean here? I was assuming `git show`, but that doesn't seem to have a `-v` parameter.

`git commit` has a `-v` option that adds the diff to the bottom of the commit message template so you can see it while you write the message.

Unfortunately, these days, I am getting a lot of PRs where nobody has read the code, which came straight out of a robot. This makes me really angry.

I self review but I don’t write comments. I simply fix the code as I see the problems that I find self reviewing.

Yep. Self-reviewing your own PRs is a large boost to both yourself and the team, and often one of the first things I encourage new-ish developers to do.

- 90% of the time when you self-review your own PR, you're going to spot a bug or some incorrect assumption you made along the way. Or you'll see an opportunity to clean things up / make it better.

- Self-reviewing and annotating your reasons/thought process gives much more context to the actual reviewer, who likely only has a surface level understanding of what you're trying to do.

- It also signals to your team that you've taken the time to check your assumptions and verify you're solving the problem you say you are in the PR description.

Even when I worked for myself and had CodeRabbit help me do MRs, I still did a self-review before pushing any change to main.

Self-review is very, very helpful.

I always review my own PR before I expect someone else to, but I generally don't add comments. I just look it over and if I see something I want to fix I fix it. Adding comments for things I specifically want feedback on or am unsure about seems like a nice addition to the process though. I might start doing that too.

The self review saves the company the most money. I have this as a diagram for my team that shows the distance where the bug is found and cost.

I do it quite often and it's great, because it helps contextualise some changes that might not seem to be intuitive.

You could argue this is what commits are for, but given how people use GitHub and PRs, it gives some extra visibility.

And if you're going to use AI to assist you when writing the code I would argue this self-review step is 100% mandatory.

Yeah, I never send a PR out without reviewing each commit myself and adding GitHub comments when I think it's relevant. Sometimes a PR is clear enough that I don't feel the need to add comments, though.

I self review but I don’t add comments I just fix the problems that I find. I should add clarifying comments.

I've done this and strongly recommend.

Yep!

Adding context to both your commits and your code review tools pull requests / merge requests makes everyone's lives better. Including future you, who inevitably is looking at the PR or commit in the future due to an incident caused by said change.

I have been following this personal rule for well over a decade, and have never regretted it.

I often do this. It's a great way to highlight the areas you want a reviewer to focus on, the areas you are least happy about and want some collab. You can't always get collab pre-review, as you're writing. Plus you want to write it down and move on. Then you can async edit when the feedback comes. Not unlike a prose writing process.

Reply on news.ycombinator.com