Code Review

  • Improve code quality
  • Find bugs early
  • Uniform code Specification
  • Improve team members’ coding skills

In short, problem finding (code specifications, potential defects, bugs, code design, etc.) in the early stage leads to developer technical communication and staff growth in the later stage

How to develop

  • Code specification: Clear Coding rules
  • Check list: check key points according to business characteristics
  • Summary optimization: transparent problem, continuous optimization
  • Incentives: Stimulate subjective initiative

To carry out the way

  • Mandatory & non-mandatory
  • Online communication (Group Review) & Offline Meeting (Group Review)
  • Small fragments & large modules
  • Before & after release
  • High frequency & low frequency

The resistance factor

  1. The leader or key members of the team disagree
  2. In order to cope
  3. Excuse being lazy with too much demand and no time

Tissue types

  • Team review, usually by module leader or project leader, is relatively frequent, at least once a day
  • Team review, usually the entire team review code, with the team leader taking the lead, at least once a week, given the company situation

Review the content

Harmonize team code style and programming specifications

Static code inspection tools

  1. Java classes: Checkstyle, FindBugs, PMD, Infer, etc
  2. JavaScript classes: JSLint, ESLint, etc
  3. Object-c: OCLint, Clang Static Analyzer, Infer, etc
  4. **C#** class: StyleCode, etc

Some coding specifications to refer to (github.com/Kristories/…)

Found “Bad Smell” code and bugs

Related books: Refactoring – Improving the Design of Existing Code

Good experience of team members

  • What writing might cause poor performance?
  • Which interface should be used with caution?
  • What design practices should be avoided?
  • What habits are likely to cause memory leaks? …

Features that developers felt were not designed properly due to time constraints

  • Functional imperfection
  • Lack of design
  • The code has a better implementation scheme
  • Focus on readability of project code

In summary, the code conforms to the code style specification agreed upon by the team, the code is appropriate for the business it implements, the code is safe, the code performs, and the code is friendly to subsequent developers, i.e. easy to maintain

Matters needing attention

  1. GitLab can set up protection for the Master and Develop branches, and developers cannot push code to these branches, only through PR/MR.
  2. This can be done by setting up git pre-commit hooks to prevent non-conforming code from committing to the repository.
  3. Coordinate CI checks as the first step in build.
  4. The user roles are owner/host/developer/reporter/visitor, where only the owner and host have the permission to review code and merge code.
  5. Note that at least two members of the team have the authority to review and merge the code, so that the code does not close due to the absence or leave of one member.
  6. The main program must be careful not to pile too much module work on itself, and must learn to allocate tasks properly, because you will also need to have the energy to review the code, which is part of the extra task.
  7. The develop branch is not allowed to commit and only merge code that has been reviewed. The Master branch is not allowed to commit and only merge code from the Develop branch.
  8. Not necessarily, the higher the title is, the more likely it is to review the code than others, and the knowledge sharing of code review is more valued. It is possible to find bugs through review, but it is not the ultimate goal. In fact, it is more important to enhance team consensus and protect team consistency.
  9. Try to avoid code review by inexperienced developers or people (even senior engineers) who are new to the company and unfamiliar with the business.
  10. If you can write unit tests as much as possible, they may not cover all of them. If you are pressed for time, you can only do the key modules.
  11. Submit the PR/MR and be sure to inform relevant people on IM for review, such as the project leader or module leader.
  12. Control the team review time, half an hour to an hour, preferably not more than an hour, 30-40 minutes is appropriate, the project leader specific control.
  13. According to the situation of the company, team review is appropriate at least once a week.
  14. Review may take several times to be allowed into the code, which means your code may need to be changed several times to get it right.
  15. Avoid code accumulation, resulting in a review of a large number of code, on the one hand, it is easy to review, but also a waste of time, resulting in unsatisfactory results.
  16. It is suggested that one person make a record, summarize the improvement points of each review in the form of a list and clearly send to all participants.

conclusion

Due to the tight schedule and rapid change of requirements, if you don’t know why you need to conduct Code Review, it will be easy to compromise when encountering obstacles. Gradually, the Code Review will go out of shape and eventually become a mere formality. On the other hand, when we hit a roadblock and the review code didn’t go well, we solved the problem with a positive attitude. Code Review will affect the development efficiency. In fact, the pursuit of high-quality Code itself reduces the partial development efficiency, but in the long run, the Code written in this way will be more robust, with no or few “weird” bugs, reducing the cost of later maintenance.

Therefore, there is no problem with Code Review itself. In fact, people are prone to problems.