Site updated: 2019-10-06 03:05:10

KENTHD8mg · Oct 5, 2019 · 5eb1888 · 5eb1888
1 parent 72151b4
commit 5eb1888
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 15 deletions.
diff --git a/2019/10/06/codeReview-best-practices/index.html b/2019/10/06/codeReview-best-practices/index.html
@@ -1,5 +1,5 @@
-<!DOCTYPE html><html><head><meta name="generator" content="Hexo 3.9.0"><meta http-equiv="content-type" content="text/html; charset=utf-8"><meta content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=0" name="viewport"><meta content="yes" name="apple-mobile-web-app-capable"><meta content="black-translucent" name="apple-mobile-web-app-status-bar-style"><meta content="telephone=no" name="format-detection"><meta name="description"><title>CodeReview实践的目的 | KENTHD8mg</title><link rel="stylesheet" type="text/css" href="/css/normalize.css"><link rel="stylesheet" type="text/css" href="/css/highlight.css"><link rel="stylesheet" type="text/css" href="/css/very-simple.css"><link rel="stylesheet" type="text/css" href="//cdn.bootcss.com/font-awesome/4.5.0/css/font-awesome.min.css"><link rel="Shortcut Icon" type="image/x-icon" href="/favicon.ico"><link rel="alternate" type="application/atom+xml" href="/atom.xml"></head><body><!-- include the sidebar--><!-- include ./includes/sidebar.jade--><!-- Blog title and subtitle--><header><div class="container header"><a id="logo" href="/." class="title">KENTHD8mg</a><span class="subtitle">as a excited newbird geeker forever</span><label id="toggle-menu" for="menu" onclick><i class="fa fa-bars"></i></label></div></header><!-- use checkbox hack for toggle nav-bar on small screens--><input id="menu" type="checkbox"><!-- Navigation Links--><nav id="nav"><div class="container"><a href="/" class="sidebar-nav-item active">Home</a><a href="/archives" class="sidebar-nav-item">Archives</a></div></nav><div id="header-margin-bar"></div><!-- gallery that comes before the header--><div class="wrapper"><div class="container post-header"><h1>CodeReview实践的目的</h1></div></div><div class="wrapper"><div class="container meta"><div class="post-time">2019-10-06</div><div class="post-categories"><a class="post-category-link" href="/categories/敏捷开发实践/">敏捷开发实践</a>><a class="post-category-link" href="/categories/敏捷开发实践/codereview/">codereview</a></div><div class="post-tags"><a class="post-tag-link" href="/tags/敏捷开发实践/">敏捷开发实践</a></div></div></div><article><div class="container post"><p><a name="8f9rJ"></a><br>在敏捷开发实践中，CodeReview作为代码检查过程中比较重要的一个环节，在各团队里都有不错的实践。打算结合自己团队的实践写一篇关于CodeReview完整的最佳实践的建议，发现周末时间不够，就改成CodeReview实践中的几个问题，还是不够最后改成CodeReview实践的目的，把丰满的理想先暂存在心里，有空补上。<br><a name="Qimse"></a></p>
-<h2 id="目的和意义"><a href="#目的和意义" class="headerlink" title="目的和意义"></a>目的和意义</h2><p>团队在开始CodeReview实践过程中，容易遗漏在团队里对CodeReview的目标建立统一的认识。为了避免拿到代码就开始缺乏目标的尽量找缺陷的游戏，我们这里试图去建立一些准则，让我们的CodeReview实践有据可依。<br><a name="bMgFv"></a></p>
+<!DOCTYPE html><html><head><meta name="generator" content="Hexo 3.9.0"><meta http-equiv="content-type" content="text/html; charset=utf-8"><meta content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=0" name="viewport"><meta content="yes" name="apple-mobile-web-app-capable"><meta content="black-translucent" name="apple-mobile-web-app-status-bar-style"><meta content="telephone=no" name="format-detection"><meta name="description"><title>CodeReview实践的目的 | KENTHD8mg</title><link rel="stylesheet" type="text/css" href="/css/normalize.css"><link rel="stylesheet" type="text/css" href="/css/highlight.css"><link rel="stylesheet" type="text/css" href="/css/very-simple.css"><link rel="stylesheet" type="text/css" href="//cdn.bootcss.com/font-awesome/4.5.0/css/font-awesome.min.css"><link rel="Shortcut Icon" type="image/x-icon" href="/favicon.ico"><link rel="alternate" type="application/atom+xml" href="/atom.xml"></head><body><!-- include the sidebar--><!-- include ./includes/sidebar.jade--><!-- Blog title and subtitle--><header><div class="container header"><a id="logo" href="/." class="title">KENTHD8mg</a><span class="subtitle">as a excited newbird geeker forever</span><label id="toggle-menu" for="menu" onclick><i class="fa fa-bars"></i></label></div></header><!-- use checkbox hack for toggle nav-bar on small screens--><input id="menu" type="checkbox"><!-- Navigation Links--><nav id="nav"><div class="container"><a href="/" class="sidebar-nav-item active">Home</a><a href="/archives" class="sidebar-nav-item">Archives</a></div></nav><div id="header-margin-bar"></div><!-- gallery that comes before the header--><div class="wrapper"><div class="container post-header"><h1>CodeReview实践的目的</h1></div></div><div class="wrapper"><div class="container meta"><div class="post-time">2019-10-06</div><div class="post-categories"><a class="post-category-link" href="/categories/敏捷开发实践/">敏捷开发实践</a>><a class="post-category-link" href="/categories/敏捷开发实践/codereview/">codereview</a></div><div class="post-tags"><a class="post-tag-link" href="/tags/敏捷开发实践/">敏捷开发实践</a></div></div></div><article><div class="container post"><p>在敏捷开发实践中，CodeReview作为代码检查过程中比较重要的一个环节，在各团队里都有不错的实践。打算结合自己团队的实践写一篇关于CodeReview完整的最佳实践的建议，发现周末时间不够，就改成CodeReview实践中的几个问题，还是不够最后改成CodeReview实践的目的，把丰满的理想先暂存在心里，有空补上。</p>
+<h2 id="目的和意义"><a href="#目的和意义" class="headerlink" title="目的和意义"></a>目的和意义</h2><p>团队在开始CodeReview实践过程中，容易遗漏在团队里对CodeReview的目标建立统一的认识。为了避免拿到代码就开始缺乏目标的尽量找缺陷的游戏，我们这里试图去建立一些准则，让我们的CodeReview实践有据可依。</p>
 <h3 id="持续保持代码的健康"><a href="#持续保持代码的健康" class="headerlink" title="持续保持代码的健康"></a>持续保持代码的健康</h3><blockquote>
 <p>The primary purpose of code review is to make sure that the overall code health of  code base is improving over time. All of the tools and processes of code review are designed to this end.</p>
 </blockquote>
@@ -26,7 +26,6 @@ <h3 id="持续保持代码的健康"><a href="#持续保持代码的健康" clas
 <blockquote>
 <p>Technical facts and data overrule opinions and personal preferences.<br>遵循技术事实和数据代替个人观点和个人偏好<br>On matters of style, the style guide is the absolute authority. Any purely style point (whitespace, etc.) that is not in the style guide is a matter of personal preference. The style should be consistent with what is there. If there is no previous style, accept the author’s.<br>建立样式文档，编码规范，必须遵循文档规范，如果没有文档规范遵循原有上下文的代码规范，如果没有的就接受developer的代码<br>Aspects of software design are almost never a pure style issue or just a personal preference. They are based on underlying principles and should be weighed on those principles, not simply by personal opinion. Sometimes there are a few valid options. If the author can demonstrate (either through data or based on solid engineering principles) that several approaches are equally valid, then the reviewer should accept the preference of the author. Otherwise the choice is dictated by standard principles of software design.<br>遵循基本设计原则<br>If no other rule applies, then the reviewer may ask the author to be consistent with what is in the current codebase, as long as that doesn’t worsen the overall code health of the system.<br>与当前代码库保持一致</p>
 </blockquote>
-<p><a name="Uks3Z"></a></p>
 <h3 id="建设工程师文化"><a href="#建设工程师文化" class="headerlink" title="建设工程师文化"></a>建设工程师文化</h3><blockquote>
 <p>Code review can have an important function of teaching developers something new about a language, a framework, or general software design principles. It’s always fine to leave comments that help a developer learn something new. Sharing knowledge is part of improving the code health of a system over time. Just keep in mind that if your comment is purely educational, but not critical to meeting the standards described in this document, prefix it with “Nit: “ or otherwise indicate that it’s not mandatory for the author to resolve it in this CL.</p>
 </blockquote>
@@ -38,12 +37,12 @@ <h3 id="建设工程师文化"><a href="#建设工程师文化" class="headerlin
 <blockquote>
 <p>Code Review协作过程：<br>a）先由代码的开发人员向检查人员进行大体的介绍，包括设计思想、数据结构、程序代码结构介绍等。<br>b）双方进行讨论、交流。<br>c）检查人员单独进一步进行Code Review，并记录Review结果和建议。<br>d）由检查人员和开发人员一起，检查人员反馈Code Review结果，并和开发人员一起讨论改进方法，重构。<br>e）最后把可重用的Code Review的经验总结编码规范。便于整个团队复用经验。</p>
 </blockquote>
-<p>这个过程中，需要将共识的经验总结成编码规范落成文档容易被遗漏，避免在CodeReview过程中不必要的冲突。<br><a name="s8GUg"></a></p>
-<h3 id="及早发现潜在缺陷与-BUG，降低事故成本"><a href="#及早发现潜在缺陷与-BUG，降低事故成本" class="headerlink" title="及早发现潜在缺陷与 BUG，降低事故成本"></a>及早发现潜在缺陷与 BUG，降低事故成本</h3><p>很多团队在进行CodeReview的目的是为了避免bug的产生，作为代码质量的一种手段，有线上bug，reviewer跟developer同责任。其实有过CodeReview实践的同学都有一个共识，CodeReview并不能避免bug，甚至不是一种有效减少bug的手段。所以避免bug，将CodeReview作为一种质量保障过程，不是一个好的实践。<br>如果reviewer需要对于代码中的业务逻辑负责，会出现团队里能review代码的很少，CodeReview参与度越来越少的局面，违背了CodeReview的团队参与，代码协作的根本目的。更详细的讨论可以参见引用文档</p>
+<p>这个过程中，需要将共识的经验总结成编码规范落成文档容易被遗漏，避免在CodeReview过程中不必要的冲突。</p>
+<h3 id="及早发现潜在缺陷与-BUG，降低事故成本"><a href="#及早发现潜在缺陷与-BUG，降低事故成本" class="headerlink" title="及早发现潜在缺陷与 BUG，降低事故成本"></a>及早发现潜在缺陷与 BUG，降低事故成本</h3><p>很多团队在进行CodeReview的目的是为了避免bug的产生，作为代码质量的一种手段，有线上bug，reviewer跟developer同责任。其实有过CodeReview实践的同学都有一个共识，CodeReview并不能避免bug，甚至不是一种有效减少bug的手段。所以避免bug，将CodeReview作为一种质量保障过程，不是一个好的实践。如果reviewer需要对于代码中的业务逻辑负责，会出现团队里能review代码的很少，CodeReview参与度越来越少的局面，违背了CodeReview的团队参与，代码协作的根本目的。更详细的讨论可以参见引用文档</p>
 <blockquote>
 <p>发现代码中的业务逻辑错误<br>业务逻辑指的是代码开发的功能是否符合业务需求，如一个加法函数，检查其是否真的实现了加法的功能。<br>笔者了解的一些敏捷团队中，把发现代码的业务逻辑错误当做目标和内容，但往往效果都不是很好，基本都是从形式上泛泛检查一番。原因有两个：<br>1．业务逻辑的检查是从需求到代码的全方位检查，需要花费大量时间，投入产出比失衡。<br>2．业务逻辑的检查和业务需求紧密关联，已经超出了检查人员的能力范围（一般Code Review是开发人员，不是业务人员）。<br>笔者认为，发现逻辑错误，不应该是Code Review 的目的和内容。应该是Unit Test，功能测试，集成测试的目的。从投入产出比考虑，不应该花费太多时间在Code Review 阶段去进行逻辑错误检查。</p>
 </blockquote>
-<p>代码中的逻辑，除了业务逻辑，还应该包括技术逻辑。技术逻辑就是实现逻辑， 比如数据库连接打开是否忘记关闭，是否正确使用线程，Exception 处理，密码是否加密存储，并发锁是否在所有分支释放等。虽然发现代码错误不是CodeReview的主要职责，CodeReviewe不能代替测试。但是可以把这些最常出现错误的地方，而且是测试不容易发现的地方，作为Code中的“地雷区”。这些“地雷区”在CodeReview中是值得花费一些时间进行维护和检查的。建议，在整个团队中维护并共享“地雷区”注意事项列表，以及统一的处理方式和机制。并在编码和CodeReview过程中都按照团队的最佳实践进行。也可以用代码规约工具和bug finder工具扫描。在一定程度上降低这些bug带来的事故成本。<br><a name="h2Lye"></a></p>
+<p>代码中的逻辑，除了业务逻辑，还应该包括技术逻辑。技术逻辑就是实现逻辑， 比如数据库连接打开是否忘记关闭，是否正确使用线程，Exception 处理，密码是否加密存储，并发锁是否在所有分支释放等。虽然发现代码错误不是CodeReview的主要职责，CodeReviewe不能代替测试。但是可以把这些最常出现错误的地方，而且是测试不容易发现的地方，作为Code中的“地雷区”。这些“地雷区”在CodeReview中是值得花费一些时间进行维护和检查的。建议，在整个团队中维护并共享“地雷区”注意事项列表，以及统一的处理方式和机制。并在编码和CodeReview过程中都按照团队的最佳实践进行。也可以用代码规约工具和bug finder工具扫描。在一定程度上降低这些bug带来的事故成本。</p>
 <h2 id="CodeReview我们应该找什么"><a href="#CodeReview我们应该找什么" class="headerlink" title="CodeReview我们应该找什么"></a>CodeReview我们应该找什么</h2><p>先把google的建议checklist放出来参考</p>
 <blockquote>
 <ul>
@@ -61,13 +60,11 @@ <h2 id="CodeReview我们应该找什么"><a href="#CodeReview我们应该找什
 <li>The code conforms to our style guides.</li>
 </ul>
 </blockquote>
-<p><a name="uZNxA"></a></p>
-<h2 id="不可避免的冲突"><a href="#不可避免的冲突" class="headerlink" title="不可避免的冲突"></a>不可避免的冲突</h2><p>在CodeReview过程中，冲突是不可避免的，每个人的职责不同，角度不同，重点不同都会做不同取舍，一个常见的情况，一个developer对修改的代码完成度相对较低最小实现完成即可，但对于一个项目管理成员来说希望修改有比较高的完成度最好整条链路梳理一遍。不违背前节原则的情况下，为了不影响整个代码往前推进，reviewer应该接受这样的代码，但也应该让reviewer有自由评论的权限，有一个好的实践是可以把review评论分成必须修改非必须修改两部分，在上述原则的前提下，认为有better的实现的代码可以标记成非必须修改review评论来给developer提供建议，这样既有利于知识的分享传播也不会产生不必要的冲突阻塞开发进程。<br>还有一个好的实践，当冲突发生时，我们在讨论代码的时候，参照stack overflow的原则，陈述观点是应该基于事实、资料文档、专业知识、数据不应该只发表观点。</p>
+<h2 id="不可避免的冲突"><a href="#不可避免的冲突" class="headerlink" title="不可避免的冲突"></a>不可避免的冲突</h2><p>在CodeReview过程中，冲突是不可避免的，每个人的职责不同，角度不同，重点不同都会做不同取舍，一个常见的情况，一个developer对修改的代码完成度相对较低最小实现完成即可，但对于一个项目管理成员来说希望修改有比较高的完成度最好整条链路梳理一遍。不违背前节原则的情况下，为了不影响整个代码往前推进，reviewer应该接受这样的代码，但也应该让reviewer有自由评论的权限，有一个好的实践是可以把review评论分成必须修改非必须修改两部分，在上述原则的前提下，认为有better的实现的代码可以标记成非必须修改review评论来给developer提供建议，这样既有利于知识的分享传播也不会产生不必要的冲突阻塞开发进程。还有一个好的实践，当冲突发生时，我们在讨论代码的时候，参照stack overflow的原则，陈述观点是应该基于事实、资料文档、专业知识、数据不应该只发表观点。</p>
 <blockquote>
 <p>Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise.</p>
 </blockquote>
-<p><a name="PMyrz"></a></p>
-<h2 id="流程和工具"><a href="#流程和工具" class="headerlink" title="流程和工具"></a>流程和工具</h2><p>PULLREQUEST：<a href="https://www.jianshu.com/p/c7a16bd17b17" target="_blank" rel="noopener">https://www.jianshu.com/p/c7a16bd17b17</a><br>AONE：<a href="https://www.atatech.org/articles/130692?spm=ata.13269325.0.0.2bde49faR9EhMf" target="_blank" rel="noopener">https://www.atatech.org/articles/130692?spm=ata.13269325.0.0.2bde49faR9EhMf</a><br><a name="11"></a></p>
+<h2 id="流程和工具"><a href="#流程和工具" class="headerlink" title="流程和工具"></a>流程和工具</h2><p>PULLREQUEST：<a href="https://www.jianshu.com/p/c7a16bd17b17" target="_blank" rel="noopener">https://www.jianshu.com/p/c7a16bd17b17</a></p>
 <h2 id="参考文章"><a href="#参考文章" class="headerlink" title="参考文章"></a>参考文章</h2><p><a href="https://google.github.io/eng-practices/review/reviewer/" target="_blank" rel="noopener">How to do a code review</a><br><a href="https://google.github.io/eng-practices/review/reviewer/standard.html" target="_blank" rel="noopener">The Standard of Code Review</a><br><a href="https://google.github.io/eng-practices/review/reviewer/looking-for.html" target="_blank" rel="noopener">What to look for in a code review</a><br><a href="https://google.github.io/eng-practices/review/developer/handling-comments.html" target="_blank" rel="noopener">How to handle reviewer comments</a><br>敏捷开发中的Code Review:《程序员》杂志<br>持续集成：《敏捷开发知识体系》<br><a href="https://www.jianshu.com/p/f79c4e948954" target="_blank" rel="noopener">CodeReview规范</a></p>
 </div><!-- comment system--><div class="container"><hr></div></article><footer id="footer"><div class="container"><div class="bar"><div class="social"><a href="/atom.xml" target="_blank"><i class="fa fa-rss"></i></a></div><div class="footer">© 2019 <a href="/" rel="nofollow">KENT</a>. Powered by <a rel="nofollow" target="_blank" href="https://hexo.io">Hexo</a>. Theme <a target="_blank" href="https://github.com/lotabout/very-simple">very-simple</a>.</div></div></div></footer><link rel="stylesheet" type="text/css" href="//cdn.bootcss.com/fancybox/2.1.5/jquery.fancybox.css"><script src="//cdn.bootcss.com/jquery/2.0.3/jquery.min.js"></script><script src="//cdn.bootcss.com/fancybox/2.1.5/jquery.fancybox.pack.js"></script><script>$(document).ready(function() {
     $(".fancybox").fancybox();