![[personal profile]](https://www.dreamwidth.org/img/silk/identity/user.png){kind=small}
Code review
Oct. 29th, 2018 04:46 pmIn our team we do code reviews a lot.
We code review everything: C#, SQL, ElasticSearch, XML, HTML, javascript, powershell, linux shell scripts, and manual installation instructions.
The main reasons to do code review:
1) Knowledge exchange
- Code reviewer learns from code he reviews.
- Coder learns from code reviewer's feedback.
- All team members learn from code review discussion.
2) Bug fixing
Code reviews detect a lot of bugs (before they damage our production environment).
We do code review multiple times per single commit:
1) Developer reviews diff of his own code before he makes his original commit.
2) Code reviewer reviews commit.
Single commit may have several code reviewers (if several developers on our team consider that commit to be relevant to their interest).
3) In the future, when we maintain our code (need to extend or fix a bug or just recall why code does what it does) -- we do svn blame for line in question and review commit again.
Because code review is so important and because we do code review multiple times -- we try to optimize code commit for code review.
That means we try to minimize time that is needed for code review:
1) We try to write clear "commit comment" explaining the purpose of each commit.
2) We try to reduce the size of individual commit.
That means - commit separate features in a separate commits (do not mix multiple features in a separate commit).
We commit refactorings (that we need to do in order to correctly implement feature) separately from actual feature implementation (it is much easier to review code diff if you know that this code is only a refactoring did not change functionality).
We try to separate refactorings from each other too.
It is much easier for code reviewer to understand smaller commits.
Smaller commit size is also easier to explain in "commit comment" (what is the purpose of that commit).
3) We understand that it is OK to spend more time on coding and explaining commit -- in order to reduce code review time.
---
Inspired by I hate code review discussion on Hacker News.
We code review everything: C#, SQL, ElasticSearch, XML, HTML, javascript, powershell, linux shell scripts, and manual installation instructions.
The main reasons to do code review:
1) Knowledge exchange
- Code reviewer learns from code he reviews.
- Coder learns from code reviewer's feedback.
- All team members learn from code review discussion.
2) Bug fixing
Code reviews detect a lot of bugs (before they damage our production environment).
We do code review multiple times per single commit:
1) Developer reviews diff of his own code before he makes his original commit.
2) Code reviewer reviews commit.
Single commit may have several code reviewers (if several developers on our team consider that commit to be relevant to their interest).
3) In the future, when we maintain our code (need to extend or fix a bug or just recall why code does what it does) -- we do svn blame for line in question and review commit again.
Because code review is so important and because we do code review multiple times -- we try to optimize code commit for code review.
That means we try to minimize time that is needed for code review:
1) We try to write clear "commit comment" explaining the purpose of each commit.
2) We try to reduce the size of individual commit.
That means - commit separate features in a separate commits (do not mix multiple features in a separate commit).
We commit refactorings (that we need to do in order to correctly implement feature) separately from actual feature implementation (it is much easier to review code diff if you know that this code is only a refactoring did not change functionality).
We try to separate refactorings from each other too.
It is much easier for code reviewer to understand smaller commits.
Smaller commit size is also easier to explain in "commit comment" (what is the purpose of that commit).
3) We understand that it is OK to spend more time on coding and explaining commit -- in order to reduce code review time.
---
Inspired by I hate code review discussion on Hacker News.
