The Art of code review: the Dropbox Story

Each line of code in the Dropbox IOS app is a bug or feature task that starts with being added to Maniphest, and Maniphest is our task management system. When an engineer takes on a task on top, the corresponding responsibility is given to him before he can begin writing the code. Phabricator This platform contains our code review tool, which has a lot of good functionality, but it does not work well in the mutual collaboration between the evaluation objects. To compensate for this, our engineers need to know who is reviewing their tasks before starting their work [1]. For the engineers under review, this ensures that there is a rubber duck in their team that knows the background and reason for some of the changes to the code in the project and assists in the design decisions of the code. For reviewers, this helps them take some of the changes into their development cycle assessment, which helps to make the development cycle assessment accurate. If there is no accident, our experience will tell us that planning well in advance can effectively avoid duplication of effort in the process of reviewing code. Planning for changes in a project can be as simple as communicating in front of a whiteboard, or as deep as writing a constructive document. It all depends on our own choice.

[1] Everyone in my team has to review the code. The new colleague will be assigned some less code before it can independently review the larger tasks.

[2] Even so, whenever a new member joins, it is always inevitable to start a debate about the use of property or Ivar.

When the task is at a certain stage, our engineers are likely to make some obviously unreasonable or unpopular decisions. The best time to capture this mentality is to take place at this moment-preparing for a future explanation of the reviewer. To explain these changes, it's easier said than done, and our engineers are encouraged to use//TODO，//HAXAnd//FIXMETo write comments in the code.//TODOAnd//FIXMEIt can literally be understood, although the latter generates a compilation warning, and must be resolved before the next release.//HAXThis note is a bit of an interesting place. [3] We use it to label the bug that bypasses Apple's API but is not easy to see at a glance. [5] Our comments will be written on the date and the name of the person who wrote the comment [4], and in many cases we are always grateful for these additional contexts.

[3] annotations are usually links to third-party sources or radar, as well as special repro steps.

[4] Like//hax: (ASHLEYNH) 2015-03-09

The Art of code review: the Dropbox Story