diff --git a/sources/tech/20141004 Practical Lessons in Peer Code Review.md b/sources/tech/20141004 Practical Lessons in Peer Code Review.md deleted file mode 100644 index 3576dd0e98..0000000000 --- a/sources/tech/20141004 Practical Lessons in Peer Code Review.md +++ /dev/null @@ -1,138 +0,0 @@ -johnhoow translating... -# Practical Lessons in Peer Code Review # - -Millions of years ago, apes descended from the trees, evolved opposable thumbs and—eventually—turned into human beings. - -We see mandatory code reviews in a similar light: something that separates human from beast on the rolling grasslands of the software -development savanna. - -Nonetheless, I sometimes hear comments like these from our team members: - -"Code reviews on this project are a waste of time." -"I don't have time to do code reviews." -"My release is delayed because my dastardly colleague hasn't done my review yet." -"Can you believe my colleague wants me to change something in my code? Please explain to them that the delicate balance of the universe will -be disrupted if my pristine, elegant code is altered in any way." - -### Why do we do code reviews? ### - -Let us remember, first of all, why we do code reviews. One of the most important goals of any professional software developer is to -continually improve the quality of their work. Even if your team is packed with talented programmers, you aren't going to distinguish -yourselves from a capable freelancer unless you work as a team. Code reviews are one of the most important ways to achieve this. In -particular, they: - -provide a second pair of eyes to find defects and better ways of doing something. -ensure that at least one other person is familiar with your code. -help train new staff by exposing them to the code of more experienced developers. -promote knowledge sharing by exposing both the reviewer and reviewee to the good ideas and practices of the other. -encourage developers to be more thorough in their work since they know it will be reviewed by one of their colleagues. - -### Doing thorough reviews ### - -However, these goals cannot be achieved unless appropriate time and care are devoted to reviews. Just scrolling through a patch, making sure -that the indentation is correct and that all the variables use lower camel case, does not constitute a thorough code review. It is -instructive to consider pair programming, which is a fairly popular practice and adds an overhead of 100% to all development time, as the -baseline for code review effort. You can spend a lot of time on code reviews and still use much less overall engineer time than pair -programming. - -My feeling is that something around 25% of the original development time should be spent on code reviews. For example, if a developer takes -two days to implement a story, the reviewer should spend roughly four hours reviewing it. - -Of course, it isn't primarily important how much time you spend on a review as long as the review is done correctly. Specifically, you must -understand the code you are reviewing. This doesn't just mean that you know the syntax of the language it is written in. It means that you -must understand how the code fits into the larger context of the application, component or library it is part of. If you don't grasp all the -implications of every line of code, then your reviews are not going to be very valuable. This is why good reviews cannot be done quickly: it -takes time to investigate the various code paths that can trigger a given function, to ensure that third-party APIs are used correctly -(including any edge cases) and so forth. - -In addition to looking for defects or other problems in the code you are reviewing, you should ensure that: - -All necessary tests are included. -Appropriate design documentation has been written. -Even developers who are good about writing tests and documentation don't always remember to update them when they change their code. A -gentle nudge from the code reviewer when appropriate is vital to ensure that they don't go stale over time. - -### Preventing code review overload ### - -If your team does mandatory code reviews, there is the danger that your code review backlog will build up to the point where it is -unmanageable. If you don't do any reviews for two weeks, you can easily have several days of reviews to catch up on. This means that your -own development work will take a large and unexpected hit when you finally decide to deal with them. It also makes it a lot harder to do -good reviews since proper code reviews require intense and sustained mental effort. It is difficult to keep this up for days on end. - -For this reason, developers should strive to empty their review backlog every day. One approach is to tackle reviews first thing in the -morning. By doing all outstanding reviews before you start your own development work, you can keep the review situation from getting out of -hand. Some might prefer to do reviews before or after the midday break or at the end of the day. Whenever you do them, by considering code -reviews as part of your regular daily work and not a distraction, you avoid: - -Not having time to deal with your review backlog. -Delaying a release because your reviews aren't done yet. -Posting reviews that are no longer relevant since the code has changed so much in the meantime. -Doing poor reviews since you have to rush through them at the last minute. - -### Writing reviewable code ### - -The reviewer is not always the one responsible for out-of-control review backlogs. If my colleague spends a week adding code willy-nilly -across a large project then the patch they post is going to be really hard to review. There will be too much to get through in one session. -It will be difficult to understand the purpose and underlying architecture of the code. - -This is one of many reasons why it is important to split your work into manageable units. We use scrum methodology so the appropriate unit -for us is the story. By making an effort to organize our work by story and submit reviews that pertain only to the specific story we are -working on, we write code that is much easier to review. Your team may use another methodology but the principle is the same. - -There are other prerequisites to writing reviewable code. If there are tricky architectural decisions to be made, it makes sense to meet -with the reviewer beforehand to discuss them. This will make it much easier for the reviewer to understand your code, since they will know -what you are trying to achieve and how you plan to achieve it. This also helps avoid the situation where you have to rewrite large swathes -of code after the reviewer suggests a different and better approach. - -Project architecture should be described in detail in your design documentation. This is important anyway since it enables a new project -member to get up to speed and understand the existing code base. It has the further advantage of helping a reviewer to do their job -properly. Unit tests are also helpful in illustrating to the reviewer how components should be used. - -If you are including third-party code in your patch, commit it separately. It is much harder to review code properly when 9000 lines of -jQuery are dropped into the middle. - -One of the most important steps for creating reviewable code is to annotate your code reviews. This means that you go through the review -yourself and add comments anywhere you feel that this will help the reviewer to understand what is going on. I have found that annotating -code takes relatively little time (often just a few minutes) and makes a massive difference in how quickly and well the code can be -reviewed. Of course, code comments have many of the same advantages and should be used where appropriate, but often a review annotation -makes more sense. As a bonus, studies have shown that developers find many defects in their own code while rereading and annotating it. - -### Large code refactorings ### - -Sometimes it is necessary to refactor a code base in a way that affects many components. In the case of a large application, this can take -several days (or more) and result in a huge patch. In these cases a standard code review may be impractical. - -The best solution is to refactor code incrementally. Figure out a partial change of reasonable scope that results in a working code base and -brings you in the direction you want to go. Once that change has been completed and a review posted, proceed to a second incremental change -and so forth until the full refactoring has been completed. This might not always be possible, but with thought and planning it is usually -realistic to avoid massive monolithic patches when refactoring. It might take more time for the developer to refactor in this way, but it -also leads to better quality code as well as making reviews much easier. - -If it really isn't possible to refactor code incrementally (which probably says something about how well the original code was written and -organized), one solution might be to do pair programming instead of code reviews while working on the refactoring. - -### Resolving disputes ### - -Your team is doubtless made up of intelligent professionals, and in almost all cases it should be possible to come an agreement when -opinions about a specific coding question differ. As a developer, keep an open mind and be prepared to compromise if your reviewer prefers a -different approach. Don't take a proprietary attitude to your code and don't take review comments personally. Just because someone feels -that you should refactor some duplicated code into a reusable function, it doesn't mean that you are any less of an attractive, brilliant -and charming individual. - -As a reviewer, be tactful. Before suggesting changes, consider whether your proposal is really better or just a matter of taste. You will -have more success if you choose your battles and concentrate on areas where the original code clearly requires improvement. Say things like -"it might be worth considering..." or "some people recommend..." instead of "my pet hamster could write a more efficient sorting algorithm -than this." - -If you really can't find middle ground, ask a third developer who both of you respect to take a look and give their opinion. - --------------------------------------------------------------------------------- - -via: http://blog.salsitasoft.com/practical-lessons-in-peer-code-review/ -作者:[Matt][a] -译者:[译者ID](https://github.com/译者ID) -校对:[校对者ID](https://github.com/校对者ID) - -本文由 [LCTT](https://github.com/LCTT/TranslateProject) 原创翻译,[Linux中国](http://linux.cn/) 荣誉推出 - - diff --git a/translated/tech/20141004 Practical Lessons in Peer Code Review.md b/translated/tech/20141004 Practical Lessons in Peer Code Review.md new file mode 100644 index 0000000000..421c10cb00 --- /dev/null +++ b/translated/tech/20141004 Practical Lessons in Peer Code Review.md @@ -0,0 +1,93 @@ +# Peer Code Review的实战经验 # + +(译者:Code Review中文可以翻译成代码复查,一般由开发待review的代码的成员以外的团队成员来进行这样的工作。由于是专业术语,没有将Code review用中文代替。) + +我有时候会听到我们的团队成员这样议论: + +"项目的Code review 只是浪费时间。" +"我没有时间做Code review。" +"我的发布时间延迟了,因为我的同时还没有完成我代码的Code review。" +"你相信我的同事居然要求我对我的代码做修改吗?请跟他们说代码中的一些联系会被打断如果在我原来代码的基础之上做修改的话。" + +### 为什么要做Code review? ### + +每个专业软件开发者都有一个重要的目标:持续的提高它们的工作质量。即使你团队中都是一些优秀的程序员,你依然不能将你自己与一个有能力的自由职业者区分开来,除非你从团队的角度来工作。Code review是团队工作的一个重要的方面。尤其是: + +代码复查者(reviewer)能从他们的角度来发现问题并且提出更好的解决方案。 + +确保至少你团队的另一个其他成员熟悉你的代码,通过给新员工看有经验的开发者的代码能够某种程度上提高他们的水平。 + +公开code reviewer和reviewee的想法和经验能够促进团队间的知识的分享。 + +能够鼓励开发者将他们的工作进行的更彻底,因为他们知道他们的代码将被其他的人阅读。 + +### 在review的过程中的注意点 ### + +但是,由于Code review的时间有限,上面所说的目标未必能全部达到。就算只是想要打一个补丁,都要确保意图是正确的。如果只是将变量名改成骆驼拼写法(camelCase),不算是code review。在开发过程中进行结对编程是有益处的,它能够使两个人得到公平的锻炼。你能够在code review上花许多时间,并且仍然能够比在结对编程中使用更少的时间。 + +我的感受是,在项目开发的过程中,25%的时间应该花费在code review上。也就是说,如果开发者用两天的时间来开发一个东西,那么复查者应该使用至少四个小时来审查。 + +当然,只要你的review结果准确的话,具体花了多少时间就显得不是那么的重要。重要的是,你能够理解你看的那些代码。这里的理解并不是指你看懂了这些代码书写的语法,而是你要知道这段代码在整个庞大的应用程序,组件或者库中起着什么样的作用。如果你不理解每一行代码的作用,那么换句话说,你的code review就是没有价值的。这就是为什么好的code review不能很快完成的原因。需要时间来探讨各种各样的代码路径,让它们触发一个特定的函数,来确保第三方的API得到了正确的使用(包括一些边缘测试)。 + +为了查阅你所审查的代码的缺陷或者是其他问题,你应该确保: + +所有有必要的测试都已经被包含进去。 + +合理的设计文档已经被编写。 + +再熟练的开发者也不是每次都会记得在他们对代码改动的时候把测试程序和文档更新上去。来自reviewer的一个提醒能够使得测试用例和开发文档不会一直忘了更新。 + +### 避免code review负担太大 ### + +如果你的团队没有强制性的code review,当你的code review记录停留在无法管理的节点上时会很危险。如果你已经两周没有进行code review了,你可以花几天的时间来跟上项目的进度。这意味着你自己的开发工作会被阻断,当你想要处理之前遗留下来的code review的时候。这也会使得你很难再确保code review的质量,因为合理的code review需要长期认真的努力。最终会很难持续几天都保持这样的状态。 + +由于这个原因,开发者应当每天都完成他们的review任务。一种好办法就是将code review作为你每天的第一件事。在你开始自己的开发工作之前完成所有的code review工作,能够使你从头到尾都集中注意力。有些人可能更喜欢在午休前或午休后或者在傍晚下班前做review。无论你在哪个时间做,都要将code review看作你的工作之一并且不能分心,你要避免: + +没有足够的时间来处理你的review任务。 + +由于你的code review工作没有做完导致版本的推迟发布。 + +提交不在相关的review由于代码在你review期间已经改动太大。 + +因为你要在最后一分钟完成他们一直于review质量太差。 + +### 书写易于review的代码 ### + +有时候review没有按时完成并不都是因为代码审查者(reviewer)。如果我的同事使用一周时间在一个大工程中添加了一些乱七八糟的代码,且他们提交的补丁实在是太难以阅读。在一段中有太多的东西要浏览。这样会让人难以理解它的作用,自然会拖慢review的进度。 + +为什么将你的工作划分成一些易于管理的片段很重要有很多原因。我们使用scrum方法论(一种软件开发过程方法),因此对我们来说一个合理的单元就是一个story。通过努力将我们的工作使用story组织起来并且只是将review提交到我们正在工作的story上,这样,我们写的代码就会更加易于review。你们的可以使用其他的软件开发方法,但是目的是一样的。 + +书写易于review的代码还有其他先决条件。如果要做一些复杂的架构决策,应该让reviewer事先知道并参与讨论。这会让他们之后review你们的代码更加容易,因为他们知道你们正在试图实现什么功能并且知道你们打算如何来实现。这也避免了开发者需要在reviewer提了一个不同的或者更好的解决方案后大片的重写代码。 + +项目需要应当在设计文档中详细的描述。这对于一个项目新成员想要快速上手并且理解现有的代码来说非常重要。这从长远角度对于一个reviewer来说也非常有好处。单元测试也有助于reviewer知道一些组件是怎么使用的。 + +如果你在你的补丁中包含的第三方的代码,记得单独的提交它。当jQuery的9000行代码被插入到了项目代码的中间,毫无疑问会造成难以阅读。 + +创建易读的review代码的另一个非常重要的措施是添加相应的注释代码。这就要求你事先自己做一下review并且在一些你认为会帮助reviewer进行review的地方加上相应的注释。我发现加上注释相对与你来说往往只需要很短的时间(通常是几分钟),但是对于review来说会节约很多的时间。当然,代码注释还有其他相似的好处,应该在合理的地方使用,但往往对code review来说更重要。事实上,有研究表明,开发者在重读并注释他们代码的过程中,通常会发现很多问题。 + +### 代码大范围重构的情况 ### + +有时候,有必要重构一段代码使其能够作用于多个其他组件。若是一个大型的应用要这样做,会花费几天甚至是更多的时间,结果是生成一个诺大的补丁包。在这种情况下,进行一个标准的code review可能是不切实际的。 + +最好的方法是增量重构你的代码。找出合理范围内的一部分改变,以此为基础来重构。一旦修改和review完成,进入第二个增量。以此类推,直到整个重构完成。这种方法可能不是在所有的情况下都可行,但是尽管如此,也能避免在重构时出现大量的单片补丁。开发者使用这种方式重构可能会花去更多的时间,但这也使得代码质量更高并且之后的review会更简单。 + +如果实在是没有条件去通过增量方式重构代码(有人可能会说之前的代码书写并组织的是多么的好),一种解决方案是在重构时进行结对编程来代替code review。 + +### 解决团队成员之间的纠纷 ### + +你的团队中都是一些有能力的专家,在一些案例中,完全有可能因为对一个具体编码问题的意见的不同而产生争论。作为一个开发者,应该保持一个开发的头脑并且时刻准备着妥协,当你的reviewer更想要另一种解决方法时。不要对你的代码持有专有的态度,也不要自己持有审查的意见。因为有人会觉得你应该将一些重复的代码写入一个能够复用的函数中去,这并不意味着这是你的问题。 + +作为一个reviewer,要灵活。在提出修改建议之前,考虑你的建议是否真的更好或者只是无关紧要。如果你把力气和注意力花在那些原来的代码会明确需要改进的地方会更加成功。你应该说"它或许者的考虑..."或者"一些人建议..."而不是”我的宠物都能写一个比这个更加有效的排序方法"。 + +如果你真的决定不了,那就询问另一个你和你的reviewee都尊敬的开发者来听一下你意见并给出建议。 + +-------------------------------------------------------------------------------- + +via: http://blog.salsitasoft.com/practical-lessons-in-peer-code-review/ +作者:[Matt][a] +译者:[john](https://github.com/johnhoow) +校对:[校对者ID](https://github.com/校对者ID) + +本文由 [LCTT](https://github.com/LCTT/TranslateProject) 原创翻译,[Linux中国](http://linux.cn/) 荣誉推出 + +