Who practices code-review?

Question

code review

Share your experience, how does the development process work?

• What code is being reviewed? (whole/individual components or changes/other option)
  
• What tools do you use?
  
• How does the code get reviewed? (always on commit, create a review request manually when you need to review, another option?)
  
• How are review results used?
  
• How is it tracked that the comments made during the review have been corrected?
  

Answer 1

“What code is being reviewed?”
- Which is not fully completed or in which new features are added.
"What tools do you use?"
- As far as I understand the question, specific tools are not used. Periodically, a copy of the old project is used for comparison.
“How does the code get reviewed?”
- see point 1.
"How are the results of the review used?"
— An incomprehensible question. The results go into the current version of the project.
“How is it tracked that the comments made during the review have been corrected?”
- A diary of the work done, a diary of subsequent changes, a diary of existing bugs is kept.

Answer 2

When review was used:
-all code (more precisely, all changes + a ticket in Jira to which the code related, there were not only bugs in the tickets, but also new features) that was committed, except for the programmer who was in the "personal" branch
-differences cooled down to the reviewer (email ), if there were comments, they were corrected and sent again until the go-ahead for commit.
There are tools that automate this, but they have not been used.

Answer 3

- The diff of one or more commits is reviewed
- Crucible (http://www.atlassian.com/software/crucible/overview).
- Automation in crucible frameworks
- The reviewer leaves comments, they are discussed in the comments to the code, corrected, committed, added to the review, and so on until a consensus is reached on all issues.
- Previously, there was a hook for committing to the trunk (without the ruve id in the crucible in the commit message, it was impossible to commit the code), now we use something similar in the git.

Answer 4

- All code gets reviewed -
We use the reviewboard tool
- Before any commit, the diff must be reviewed. The reviewboard has a "Ship it" checkbox, which is set by the reviewer if he has no comments. If the diff didn't get "Ship it", then it can't be committed
- The result of the review is a commit, in the comments to which the review id is indicated. Code that has unclosed comments on the reviewboard cannot be committed. Thus, only the code in which all the comments are eliminated gets into the repository
- Tracking is very simple - after the comments are eliminated, the diff for the current review is updated, so everyone can see that their comments have been taken into account. Those. the repository gets exactly the diff that lies on the reviewboard.

Answer 5

>> What code is being reviewed? (whole / separate components or changes / another option)
Ideally, you need to review everything, in practice it turns out to view only changes.
often new functionality may require refactoring, the reviewer's task is to determine whether it should be done now at all.
>> What tools do you use?
redmine - in products where support-revision is required.
a task is a whole feature or functionality. the task has statuses, development-completed-review-testing-release-closed.
if there are jambs in the task, the status is returned back to development. Claims to the code in the comments to the problem.
Once used Redmine Code Review Plugin, but it didn't catch on
board with magnets:
the whole project is divided into tasks. Tasks are written down on pieces of paper and stick to the board. something like this:
lh3.googleusercontent.com/-69gkJdi-ZVM/TpXc7i_nA1I/AAAAAAAABPE/SctbMV08z9A/s700/doska1.jpg
(http://axshavan.blogspot.com/2011/10/3rd-day-of-agile .html) completed tasks are included in the review column.
they are taken by people who make reviews (1-2 developers, and they make reviews, if there are jambs in the task, it again falls into new ones.)
the performer and the reviewer are written on a piece of paper.
And you may not necessarily have agile. The board itself is a cool thing. Everyone sees her, everyone works with her. Nobody messes around with task statuses, etc.
>> How does the code get reviewed? (always on commit, create a review request manually when you need to review, another option?)
a person does a task, then approaches the reviewer, and says, I did it, check me. or the version with the board, simply attaches a piece of paper to the right place.
>> How are review results used?
Mistakes are corrected, if some mistake appears often and for many, then we get together and discuss how to avoid this mistake later.
>> How is it tracked that the comments made during the review have been corrected?
the developer and his reviewer agree on the result together. the developer can try to prove to the reviewer why he began to do this and not otherwise.
Since the developer-reviewer pair is not always constant, the team lead or the person performing his duties is obliged to monitor all the project code and not allow the g-code.
Why then a reviewer? It's very simple - the reviewer checks the work, actually solves the same problem in the head, checks the algorithms, checks the boundary conditions of the algorithms, looks, of course, and also monitors the quality of the code. But he is responsible for only one task, and the team leader is responsible for everything + the final result.
+ it makes sense to agree in advance on coding standards and restrictions for a particular project and in general for all projects in general.
discuss or describe somewhere:
- coding standards
- desirable TDD, DDD methodologies, design partners
- undesirable anti-partners, that is, how we don’t do it, how it’s impossible
- agreements on the project, we do it in modules, we take it out into plugins, etc. P…
The idea is that the project should be designed as if it were one person working. Everything should be on the shelves and in the same style.
something like this)

Answer 6

My team is using CodeCollaborator. Everything is put up for review before being uploaded to the general branch. The review does not end until all open defects are fixed. Sometimes a defect is not fixed immediately, but a separate task is created in the tracker (jire) stating that something needs to be improved and the defect is marked as “tracked externally” with the number of the started task and the code is uploaded. This is a common practice with code that requires refactoring, or if the defect will be fixed by another team member as part of a separate task.
We use CodeCollaborator not only for code review before uploading it to the depot, but also if you need to discuss any code / design / text document, etc.

Answer 7

What code is being reviewed?
They made a new feature or fixed a bug - modules with changes are included in the review.
What tools do you use?
The script generates a pdf with a diff of all changed files.
How does the code get reviewed?
There is a web tool for formal reviews (not only code, but also docks, etc.). A letter is sent to all those interested, dates are set, people enter their comments, the author corrects and uploads a new version, and the review goes on again, when everyone agreed (roughly speaking, ticked) - the review closes.
How are review results used?
The code must be corrected according to the comments, or the author must convince the commenter that he is wrong. A designated moderator oversees everything.
How is it tracked that the comments made during the review have been corrected?
At the end of the review, all participants see that the code has been brought into line with the comments made, the review cannot be closed without general consent, and without the review being closed, the tool that tracks changes to the main branch in the version control system will simply not allow changes to be made on the main.

Answer 8

In general, I like the process adopted in one of the previous places of work, I still use its modification:
* each feature is made in a separate repository
* a demo site is deployed for each repository, which is tested by testers
* after development is completed and approved by testers, a diff is made with fresh trunk (I personally use rdiff for this, which is then uploaded where necessary (on the old project - local Rietveld, on all my Reviewboards)
* there the code is reviewed by someone key who has the right to commit to the trunk, or just cross-review each other
* if everything is ok, then go to the trunk