Prior wrote a "written code of Four", at that time, most of the time I was happy to read your own code. Code review is a part of daily work, but the time spent is relatively limited.
First, because the recent role reversal, the second is suddenly a lot of new people. Time spent on the code review more than writing code a lot, there are some ideas and feelings, just write about it.
In general, the size of a company in Silicon Valley a bit of code review processes are fairly standard. Mode almost. All to a PR person must have at least one stamp, to merge. If you change something related to multiple projects, each project there are people who will need to stamp down. There are some particularly critical code, such as pay-related, will usually need to pay stamp group of people for the job.
NOTE:. PR (pull request, a set of changes someone tries to push to a GitHub repository Once a PR is sent, interested parties (peer engineers) can review the set of changes, discuss potential modifications, and even push follow-up commits if necessary.
Before stamp, usually can be given a variety of comments on github page, page is shared, so no one can see. Some comments are asked, the code author directly reply or explanation on the line. Some comments pointed out that the problem is the code, so the code so authors may change; or may not agree, then reply to comments, sometimes on an implementation detail, discuss comments thread can be up to more than ten. Thus for all of us to reach a consensus helpful.
Previously encountered a friend said:
& Ldquo; see the code writing sucks, passing the buck back and forth that efficiency is not high, simply rushed in to write your own. & Rdquo;
I once had a similar idea. But recently I encountered some of the things I slowly realized that this idea is no good.
The first is from the other point of view. The other bad writing, may be unfamiliar with the business, may be unfamiliar language, it may be for large infrastructure companies are not familiar with the code. If you help him / her & ldquo; write & rdquo; instead patiently pointed out that where there are problems, then the next time he / she may still not know. Not only conducive to the growth of other people, and sometimes even make people frustrated.
Then is this way scalability poor. Even if the code review will spend ten times even write your own time and effort, but it will make people understand how code should be written in the long term, this is in fact a certain extent, & ldquo; Copy & rdquo; your productivity. You can not write anything. Especially if you start slowly with the project, with new people. five daily review of the code, and write code that five people, you think long term, which is more cost-effective?
In addition, if the write code is a learning process, how to do a good code reviewer is a learning and growing process. Pit own around a difficult, difficult to see others go so far that you will be able to tell him / her where there was a pit. And he / she noted after repeated by you, the next time he / she will be noted that similar problems helped others.
This is especially profound feelings recently. Kaka Ka before the burst group after another to a lot of new people, the boss said, the point you write less code, do more review. So that almost half of the time I work days are used to view the code and write comments. Recently, however, you will find that he / she is most of the time with each other has been able to review each other well, so I better make some time to do other work.
Code review to do more, and found that there is also a certain routine, try summarized as follows:
Code formats. Many companies have coding style guideline. Everyone's convention, to avoid the company's code style is inconsistent, it does not do in order to avoid some of the & ldquo; Shall closing parenthesis separate line & rdquo; and argue needlessly, unless it is not careful, generally we are not mistaken. However, new employees are often not very familiar in this regard. This kind of problem is relatively easy to point out.
The code readability. This includes a function not too long, too long to break down. All variable names as far as possible to explain its intentions and type. For example hosting_address_hash, to see that this is the address of the landlord, and is a type of hash. Do not have too many layers of nested conditional statements or loops. Do not have too long of a boolean judgment statement. If a function, people need to see your lengthy comments to understand that this function must be reconstructed space. In addition, if there is inevitably some comments, it must ensure that the comment is accurate and fully consistent with the code.
Help code writers think he / she did not miss any corner case. Many times this is the business logic related, in particular, need help older engineers noted in all cases need to be addressed.
Error handling. This is the most common and easiest to code review someone else see the problem. For example, the following paragraph simply could not be any simple code there are at least three potential problems: params there is not a need to validate user_id and new_name two key; if you can find this user_id corresponding user; save time will not DB level will be the exception, how they deal with.
Test cases and anti-pits. Needless to say, the test cases, each code should have the test. But for some people if you can anticipate changes in code may cause the problem situation, be sure to add additional test cases to prevent this from happening. This is not an example of the reference is not good to say. How to write test cases, write an article in itself is worth the.
Small architecture. What does that mean, that is a file or a class code inside the organization. For example, if you have code duplicate sections, you should extract the public. Do not arbitrarily set constants in your code, all constants should be a uniform definition of the top of the file. What should be a private, etc.
Great architecture. This on a wider. Including file organization, the function is not to be abstract or helper lib file; Should use inheritance class; is not the entire code base and the same style, and so on.
Of course, as the case may be, there are many other needs in the code review in attention. But if you follow this checklist over again, some obvious problems can be avoided pretty close up.
In addition, the code review and write the code are the two aspects of each of us should be committed to the engineers, are indispensable. May be at work or in some stage of a project, you will spend more time in one side, but in the long view, the two skills are indispensable.
Code review work, do not have emotional. I have done one thing, a group of colleagues outside the code, others have a stamp, but I think there is a problem, so the stamp revert out, write in the comments why there is a problem, and the recommended reform law. At that time I was still a little strange feel like offending people. But later I found a colleague but very politely and accepted the thanks. My heart is very grateful to have such a work environment.
In addition, often encounter different views, in fact, two people no consensus on programming practices and review some of the comments in the convention. If the company style guideline without departing from the situation, not necessarily the other side and you have the same habits. If you really think this is better, I usually will only write & ldquo; I personally would prefer A over B, but no strong opinion & rdquo;. Or & ldquo; How about change it to A, since ... However, this is . not a merge blocker & rdquo;
Thoughtful people have not the time. More than a few pairs of eyes together to help you go over, you can greatly minimize the error code. In addition, the mutual review process can also learn a lot from each other, where programming tips and good habits. Reflect on, a lot of Java and Ruby are particularly useful feature, I have been in the process of code review is learned.