4 ms·

One of the biggest improvements Ive made and rolled out was to do my own review first. Scan through all the code from the reviewers perspective and try and see

One of the biggest improvements Ive made and rolled out was to do my own review first. Scan through all the code from the reviewers perspective and try and see where adding a comment, renaming something, or explaining a decision might be beneficial. It makes life way easier for everyone.

franzb 4y ago
Absolutely! This works even better when doing the self-review with the same tool (typically, GitHub review system) as the one a real reviewer would use. Just looking at one's code in a different environment (different font, colors, etc.) helps switching from a "programmer's mind" to a "reviewer's mind".
- AndrewDucker 4y ago
  Totally. Looking at the diff and asking "But why did I make this change? And how would I justify it?" is very helpful.
  - XorNot 4y ago
    Except this is the problem with looking at patches in Github, as opposed to as source code. Github shows you such an utterly minimal view of the overall flow of the code that it's impossible in a lot of cases to tell what the intent was or why - you don't even have function-level context for it. Compare to if you simply checked out main and the PR, and ran a meld across both directories - changes would be highlighted, but now you have to read the code in context.
- notemaker 4y ago
  I, too, need to review my code in Gitlab/Github/Gerrit in order to catch errors - somehow they're much more prominent there (like you said, I suppose it's due to adopting a "reviewer's mindset") rather than just looking at `git diff`. But if possible I would like to review my work in the terminal as a part of my workflow, not needing to context switch to the browser. Has anyone found a good solution to that? FWIW, using tmux & nvim.
  - aidos 4y ago
    I always review before committing with git diff in the terminal but I just find that once I create the PR I’ll spot something else when reviewing there. Not sure what it is but I think it’s just taking that moment to read through from a different perspective (might even be that a different ux gives that perspective). I don’t especially love the review workflow on GitHub - but I can definitely catch bugs there.
vinnymac 4y ago
This is actually a requirement on my team. If someone does not do a self review, I leave a comment explaining that I won't be reviewing the work until the decisions that were made are explained. This helps understand the justifications for the change, and prevents unnecessary feedback loops to an incredibly high degree.
- b3morales 4y ago
  In my opinion this is the purpose of a commit message. Header: short description of the change; body: details, including justification for the change. But I agree, it's also good when people preemptively make comments on their own commits in the code review interface. Question for you though: do you have a size limit on this requirement? I.e., changes that are small enough don't need to do this?
pojzon 4y ago
This often saves me a lot of time. Not only on finding simple issues but also more complex ones. Self-Review should be advertised more as a clean-code practice in the industry.
UglyToad 4y ago
I think this is about 98% of the value I get from code review, just looking at the diff in the GitHub view generally results in me spotting all the bugs and stylistic issues that would be raised in review. This makes waiting days for the actual approval all the more frustrating!
- onion2k 4y ago
  This makes waiting days for the actual approval all the more frustrating! In your next stand up raise the fact you're waiting for a review as a blocker, and watch as reviews suddenly start getting done much more quickly.
dimal 4y ago
1000x yes. When I finish a significant PR, I almost always wait until the next day to put it up. When you’ve spent 3-4 days grinding away at a problem, the urge is to just put it up and get rid of it. But when I sleep on it and look at the code from someone else’s perspective the next day, I always find a ton of easy fixes that I didn’t see at 5pm the previous day. It makes the review so much less painful for everyone.
elpakal 4y ago
100%. I’ve found that taking a step back, going on a hike, riding my bike, walking the dog or just doing anything not coding related for a little bit before I go back and review my code really helps me catch things I miss when I don’t take a break. Even better after a night’s sleep. Distance lends perspective and all that.

franzb 4y ago

Absolutely! This works even better when doing the self-review with the same tool (typically, GitHub review system) as the one a real reviewer would use. Just looking at one's code in a different environment (different font, colors, etc.) helps switching from a "programmer's mind" to a "reviewer's mind".

AndrewDucker 4y ago
Totally. Looking at the diff and asking "But why did I make this change? And how would I justify it?" is very helpful.
- XorNot 4y ago
  Except this is the problem with looking at patches in Github, as opposed to as source code. Github shows you such an utterly minimal view of the overall flow of the code that it's impossible in a lot of cases to tell what the intent was or why - you don't even have function-level context for it. Compare to if you simply checked out main and the PR, and ran a meld across both directories - changes would be highlighted, but now you have to read the code in context.
notemaker 4y ago
I, too, need to review my code in Gitlab/Github/Gerrit in order to catch errors - somehow they're much more prominent there (like you said, I suppose it's due to adopting a "reviewer's mindset") rather than just looking at `git diff`. But if possible I would like to review my work in the terminal as a part of my workflow, not needing to context switch to the browser. Has anyone found a good solution to that? FWIW, using tmux & nvim.
- aidos 4y ago
  I always review before committing with git diff in the terminal but I just find that once I create the PR I’ll spot something else when reviewing there. Not sure what it is but I think it’s just taking that moment to read through from a different perspective (might even be that a different ux gives that perspective). I don’t especially love the review workflow on GitHub - but I can definitely catch bugs there.

vinnymac 4y ago

This is actually a requirement on my team. If someone does not do a self review, I leave a comment explaining that I won't be reviewing the work until the decisions that were made are explained. This helps understand the justifications for the change, and prevents unnecessary feedback loops to an incredibly high degree.

b3morales 4y ago
In my opinion this is the purpose of a commit message. Header: short description of the change; body: details, including justification for the change. But I agree, it's also good when people preemptively make comments on their own commits in the code review interface. Question for you though: do you have a size limit on this requirement? I.e., changes that are small enough don't need to do this?

pojzon 4y ago

This often saves me a lot of time. Not only on finding simple issues but also more complex ones. Self-Review should be advertised more as a clean-code practice in the industry.

UglyToad 4y ago

I think this is about 98% of the value I get from code review, just looking at the diff in the GitHub view generally results in me spotting all the bugs and stylistic issues that would be raised in review. This makes waiting days for the actual approval all the more frustrating!

onion2k 4y ago
This makes waiting days for the actual approval all the more frustrating! In your next stand up raise the fact you're waiting for a review as a blocker, and watch as reviews suddenly start getting done much more quickly.

dimal 4y ago

1000x yes. When I finish a significant PR, I almost always wait until the next day to put it up. When you’ve spent 3-4 days grinding away at a problem, the urge is to just put it up and get rid of it. But when I sleep on it and look at the code from someone else’s perspective the next day, I always find a ton of easy fixes that I didn’t see at 5pm the previous day. It makes the review so much less painful for everyone.

elpakal 4y ago

100%. I’ve found that taking a step back, going on a hike, riding my bike, walking the dog or just doing anything not coding related for a little bit before I go back and review my code really helps me catch things I miss when I don’t take a break. Even better after a night’s sleep. Distance lends perspective and all that.