Time passed quickly. Code Review mechanism has been in operation in the front end team of Lilac Doctor for more than a year. At the beginning of April this year, we shared our team’s experience in Code Review with the front end team of DXY Garden. The front end students of various business lines gradually began to try the Code Review mechanism and have gained some results so far. It’s time to put these practical experiences into writing and communicate with more friends.

The cause of

There is no such thing as love or hatred without any reason or cause. Similarly, there is no Code Review for no reason. At the beginning, there were two people in front of Dingxiang Doctor. Basically, one person was doing the DINGxiang Doctor SPA project, and the other person was managing the background project.

At the beginning of 2017, the team changed from 2 to 3. At this time, there are mainly three front-end projects (Dingxiang Doctor SPA, Dingxiang Doctor management background and Dingxiang Doctor Hybrid App) in iteration, among which the SPA project mainly involves the cross maintenance of three people. This is when minor problems begin to show up. Such as:

  • Inconsistent coding styles
  • Some business logic written by others takes more time to learn when it is cross-maintained
  • Some low-level bugs are not discovered until the code is deployed to the test environment

To solve these problems, we decided to try Code Review. The Code of the project is hosted on the Gitlab on the Intranet of the company, so we will start to explore the Merge Request of the project in Gitlab to conduct Code Review of others’ codes.

In Q2 of 2017, we began to iterate frequently on dingxiang Doctor mini program, and at the same time, the operation team also began to put forward some operational REQUIREMENTS for H5. There are now four members of the team. With the addition of new blood, we encountered new problems:

  • The addition of new people increased the diversity of the team’s code styles
  • Redundant code for new functionality implemented without a good understanding of the existing project
  • Who is responsible for the code quality and growth of new members? (Note: This is an important point)

At this time, we are still doing Code Review, but in fact, there is no strict implementation, and there is no Code Review standard for everyone to abide by.

One thing is for sure: with the development of Clove Doctor business, these problems need to be solved, otherwise it will do great harm to both the team and its members in the long run.

In Q3 of ’17, the team was already six people. The complexity of these questions increases with each new addition. In order to solve these problems, the team decided to take Code Review as a basic system and strictly implement it.

How to do Code Review?

The premise

Before we started the strict Code Review, we determined three basic specifications.

  1. Unified Git branch model for project compliance based on project version control
  2. For JavaScript, use the uniform Eslint rules
  3. Identify uniform code specifications based on team members’ existing styles

tool

The tools used are locally sourced, still GitLab. There are two key points in the whole Code Review process in GitLab project: Merge Request (MR) and Discussion (Diss).

Based on these two points, we identified several roles:

  • Owner (Requirement leader, code change submitter, MR initiator)
  • Reviewers (MR participants, from front end team colleagues, maybe more than one person. Responsible for Review code.
  • Disser (a Reviewer. To someone who initiated Discussion on MR.

process

  1. Set settings-general-merge request settings-only allow Merge requests to be merged If all discussions are resolved).
  2. Owner in the local development environment, a branch (take a branch feature-example as an example) completes function development, and pushes the code to GitLab after full self-test.
  3. Owner Initiates MR whose target branch is Develop based on the feat-example branch. MR needs to be as detailed as possible. For example: the address of the requirements document, what modifications were made, the design and implementation ideas of a certain function, which reviewer is needed to conduct Code Review on this MR, etc. The MR template is recommended.
  4. After successfully initiating MR, the Owner should inform the Reviewers that there is any Code Review required by MR and the urgency of MR through team work.
  5. Reviewers for code review based on MR. If there are any questions about MR, comment (initiate Discussion) on the specific code on GitLab, and notify Owner of the result after the review is completed (THIS time MR passed/this time MR has N Diss). If there are Diss, the Owner needs to respond to each Diss until the status of all Diss changes to Resolved.
  6. Owner merges MR, releases codes in the test environment, and notifies relevant QA students to test. After passing the QA test, QA notifies product and designer to conduct acceptance. If the Owner is sure that the merge operation can be performed? We have two suggestions: 1. Subject to the Reviewers notice Owner 2. Careful to Reviewers’ approval for MR, as MR can be Reviewers operation on GitLab. The team currently uses the second approach.)
  7. If a problem is found during testing or acceptance, the Owner needs to modify the code and then initiate a new MR until the test environment code passes acceptance.
  8. Confirm with QA students that the code can be released to the production environment, and publish the code, and inform case students that a function has been online.

The principle of

In implementing Code Review, we have some principles to follow:

  • Code before Owner initiates MR needs to be fully self-tested
  • Code versioning should not be too granular with commit
  • Do not block others’ work and respond to others’ request for Code Review as soon as possible (this is a test of team members’ spirit of cooperation and teamwork. It also requires developers to manage their time properly and be able to drop what they’re working on and pick up where they left off.)
  • If a MR is urgent, please let us know Reviewers
  • The Owner should not delete the branch at the test collection stage unless necessary (e.g. “remove source branch when merge request is accepted.”). The Owner should wait for the branch to merge into the master branch and remove it. Avoid missing in pre-launch/test branch rebuild.
  • Regularly Review and summarize Code Review implementation (e.g. during weekly team meetings)

The border

Once you understand the Code Review process, there are some boundary cases to consider. I will write down the team’s current handling strategy for your reference.

  1. Do I need to follow the Code Review process when there are urgent online bugs over the weekend? You can skip Code Review and focus on quick bug fixes.
  2. What if a requirement (project) has a very tight development schedule? Code Review can not be carried out, and the timely launch of requirements (projects) can be guaranteed first.
  3. Do we need to conduct code review for internal projects and interest projects initiated by students in the group? The decision is made by the project Owner.
  4. What if MR encounters code conflicts? It is suggested that after code review, the Owner should merge the code locally and resolve the conflict, and then push the latest code to GitLab (at this point, MR will automatically merge on GitLab).

harvest

Frankly, it was not easy for a team that had never done a Code Review before to make this mechanism work. Especially in the early stages after deciding to start Code Review. But if the direction can be identified and the team members work together to move in the given direction, the results will be as follows:

  • Team members have a uniform code style
  • Reduce the resistance of cross-maintenance of projects
  • Enable new members to integrate into the team more quickly
  • Avoid low-level bugs in the test environment
  • Good technical communication atmosphere

To be perfect

The mechanism described above is not perfect. At present, I can think of the following points that can be optimized:

  • Optimize the coding specification (as the technology itself evolves and the team improves, the previously established coding specification will be optimized as appropriate)
  • Check List (this is actually something the team is already doing. When the business has a certain complexity, some iteration of business logic will inevitably involve more existing business, at this point, if there is a Check List, this will help the Owner and Reviewers to better conduct Code Review)
  • Incentive mechanism
  • Code test cases (primarily, business code adds test cases. The team has also started experimenting.)
  • automation

Write in the last

The Code Review mechanism used by the team was deposited in words, mainly to share with more people. If these words are helpful to some people, some teams, that’s a great thing for me. It’s also nice and grateful to receive tips on how to optimize your existing mechanics.

In addition, I also want to express that the front end team of Lilac Doctor cares very much about the growth of each team member.

I guess you can guess what I’m going to say next.

Yes, with the development of Dingxiang Doctor business, we need excellent front end students to join us and grow strong and wanton together. For more information about the team, please refer to how is the front end team of Dingxiang Doctor?

Recruitment JD

Senior/Senior front End Engineer

The job description

  • Responsible for the front-end development of the products of Dingxiang Medical (website, Web App, Hybrid App, wechat mini program, management background, node.js middle layer);
  • Complete front-end project development and maintenance with high quality and efficiency according to product requirements;
  • Optimize the front-end performance of the product to ensure that the product has high quality user experience;
  • Participated in the foundation platform construction of the front end team of DXY Garden;

qualifications

  • At least 3 years front-end working experience;
  • Proficient in HTML (HTML5), CSS (CSS3) and JavaScript (ES6/ES7);
  • Familiar with network protocol (HTTP/SSL);
  • Familiar with Webpack or rollupJS;
  • Familiar with at least one CSS preprocessor (e.g., Less, Sass, Stylus);
  • Proficient in at least one of vue. js, React.js, and AngularJS frameworks;
  • Have certain understanding and practice of front-end development specification, engineering, componentization and testing;
  • Understand and skillfully use object-oriented programming ideas, pay attention to the application of design patterns and modular development in practical projects;
  • Strong sense of responsibility, good communication skills and documentation skills;

Priority condition

  • Include your Github account or blog address on your resume;
  • Independently developed or participated in quality open source projects;
  • Experience in Hybrid App project development;
  • Have practical experience in wechat small program development;
  • Experience in node.js application development and operation in high load scenarios;
  • Familiar with TypeScript;
  • Familiar with a non-front-end programming language (e.g. Java, PHP, Python, Go);

Front-end Intern (full time)

The job description

  • Responsible for the front-end development of the products of Dingxiang Medical (website, Web App, Hybrid App, wechat mini program, management background, node.js middle layer);
  • Complete front-end project development and maintenance with high quality and efficiency according to product requirements;
  • Optimize the front-end performance of the product to ensure that the product has high quality user experience;
  • Participated in the foundation platform construction of the front end team of DXY Garden;

qualifications

  • Have passion for programming technology, expect to have rapid growth in technology;
  • Full-time internship for at least 6 months prior to graduation;
  • Proficient in HTML (HTML5), CSS (CSS3) and JavaScript (ES6/ES7);
  • Familiar with network protocol (HTTP/SSL);
  • Understand and skillfully use object-oriented programming ideas, pay attention to the application of design patterns and modular development in practical projects;
  • Strong sense of responsibility, good communication skills and documentation skills;

Priority condition

  • Include your Github account or blog address on your resume;
  • Independently developed or participated in quality open source projects;
  • Proficient in using one of vue. js, React. Js, and AngularJS frameworks;
  • Experience in Hybrid App project development;
  • Experience in node.js application development and operation in high load scenarios;
  • Familiar with TypeScript;
  • Familiar with a CSS preprocessor (e.g., Less, Sass, Stylus);
  • Familiar with a non-front-end programming language (e.g. Java, PHP, Python, Go);

Now that you have seen this, why not send an email to us to chat: [email protected].

Author: Zhiyao, Front-end engineer of Dingxiang Garden