3 ms·
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 a
by vinnymac 1y ago
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.
- tcoff91 1y agoBy self review, you mean that the developer adds comments in the code review tool? that is a great idea, I want to try this.
- DerArzt 1y agoI'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.
- bornfreddy 1y agoDo 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?
- DerArzt 1y agoThe 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).
- goosejuice 1y agoI 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.
- manwe150 1y agoWhat is `-v` mean here? I was assuming `git show`, but that doesn't seem to have a `-v` parameter.
- ljosa 1y ago`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.
- jeffbee 1y agoUnfortunately, 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.
- tcoff91 1y agoI self review but I don’t write comments. I simply fix the code as I see the problems that I find self reviewing.
- nicksergeant 1y agoYep. 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.
- t-writescode 1y agoEven 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.
- sfn42 1y agoI 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.
- brainzap 1y agoThe 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.
- ljm 1y agoI 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.
- Fishkins 1y agoYeah, 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.
- tcoff91 1y agoI self review but I don’t add comments I just fix the problems that I find. I should add clarifying comments.
- disgruntledphd2 1y agoI've done this and strongly recommend.
- vinnymac 1y agoYep! 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.
- jiveturkey 1y agoI 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.