The art of code review: The story of Dropbox, review dropbox

The art of code review: The story of Dropbox, review dropbox

Every line of code in the iOS app of Dropbox begins with a bug or function task added to Maniphest. Maniphest is our task management system. When an engineer accepts a task, the corresponding responsibility has been assigned to the engineer before he starts writing the code. The Phabricator platform contains our code review tool, which has many good features, but it is not doing well in evaluating the mutual collaboration between objects. To make up for this, our engineers need to know who reviews their tasks before starting their work [1]. For the code reviewed engineers, this ensures that there is a rubber duck in their team who knows the background and reasons for some code changes in the project, it also assists in code design decisions. For reviewers, this helps them take some changes into account in their development cycle evaluation, which helps the accuracy of the development cycle evaluation. If nothing happens, our experience will tell us that planning ahead can effectively avoid repeated efforts during code review. Planning for Project changes can be as simple as communicating before the whiteboard, or as in-depth as writing a constructive document. It all depends on our choice.

[1] everyone in my team should review the code. New colleagues will be assigned a small amount of code before they can independently review large tasks.

As the task expands, engineers need to keep in mind our code specifications. This specification is a major integration of best practices and consistency specifications. Its existence makes it easier to [2] to guess how we code it. Because this is a big project, no one in the development team can perfectly map or understand the entire project. Therefore, our engineers need to rely on the help of other engineers in the team to combine the functions of the code into a whole, which helps us understand the logic when reading the code.

[2] Even so, when a new member joins, it is inevitable that a debate on using property or ivar will be held.

When the work of this task reaches a certain stage, our engineers may make some obviously unreasonable or undesirable decisions. The best time to capture this mentality is when it happens-preparing for future explanations to reviewers. It is easy to explain these changes and our engineers are encouraged to use them.//TODO,//HAX, And//FIXMETo write comments in the code.//TODOAnd//FIXMEIt can be understood literally. Although the latter generates a compilation warning, it must be resolved before the next release.//HAXThis comment is interesting. We use it to mark the bugs used to bypass Apple's API, but it is not easy to see at a glance [3]. Our comments will write the date and the name of the annotator [4]. In the future, we will always be grateful for these additional contexts [5].

[3] The annotation is usually a third-party source or radar link, and there are special reproduction steps.

[4] For example, image // HAX :( ashleynh)