团队里每天都在做Code Review但真正把Review做出价值、让人愿意投入去做的团队说实话不多。有人把审查当成合并代码前的一道关卡有人把它当成挑刺大会也有人干脆批量点一下Approved就算交差。直到我慢慢把一套更开放的做法落地到团队里才发现代码审查这件事本身远不止找bug这么简单。open-code-review这个思路核心就是把审查从单向的检查变成双向的对话从守在合并前的关口变成贯穿开发全程的协作方式。这篇文章我会顺着这条线把我踩过的坑、验证过有效的方法、以及具体到工具和流程的细节都摊开来讲希望能给正在调整团队开发流程的朋友一些参考。1. 为什么代码审查总是流于形式先聊一个真实场景。早几年我带过一个前后端分离的项目后端四个开发前端三个开发每个迭代都排得很满。Code Review的制度是有的GitLab上的Merge Request权限也配了但实际执行就是另一回事了——功能开发到最后两天大家疯狂提交MRreviewer被之后点开页面看到上千行的diff基本只能挑几个命名问题、加两句LGTM然后点击合并。线上出过几次事故印象最深的是一次空指针代码里根本没有对上游返回的null做判断而这个问题恰恰在review的diff里明晃晃摆着就是没人看出来。后来我复盘这件事发现问题的根源不在谁的责任而在整个Review机制的设计上。我们把审查当成了发布流程中的一个checkpoint而非工程实践的一环。这种模式下审查发生在代码已经写完、功能已经联调完成之后发现问题的成本已经很高人的心理预期也变成了尽快通过而不是尽量深挖。Review被当成质量checklist来用reviewer默认作者应该已经自测过了于是看代码时搜索的是格式问题、风格问题而不是逻辑漏洞、边界条件、安全问题。没有时间预算。迭代排期从不给review留时间于是审查永远在赶工赶工出来的审查自然只能走马观花。说白了传统Review是事后把关思维而open-code-review的思路是把审查提前、拆小、开放给更多人参与并且用工具和流程引导人关注真正重要的东西。我自己更愿意把它理解成一种开放式协作实践代码不是提交之后才开始被review而是在写的过程中就处于一种随时可以被讨论的状态reviewer不是由一个人扮演而是一个流动的、有多视角参与的群体评论不是指指点点而是基于共同目标的技术对话。这么说可能有点抽象我拆成几个具体的改变来说。一是把Review从大爆炸式改成小步快跑。以前一个MR有几百上千行现在团队里约定每个MR尽量控制在300行以内超过这个规模就要拆。这个约束本身就能逼着你把任务拆细而拆细之后reviewer每次只需要面对一小段逻辑大脑的负担小很多自然愿意认真看。而且小MR的好处是上下文信息少审查者不需要在脑子里维护一个大状态机出问题的概率指数级下降。二是把唯一的reviewer扩展成Review梯队。至少两个人参与一个负责逻辑正确性一个负责兼容性和整体设计。如果改动涉及数据库或安全相关还必须拉上对应领域的同学。多人review不是为了凑人数而是因为不同背景的人看代码的注意力分布完全不同有人对边界条件敏感有人对命名和结构敏感有人对性能敏感。open的过程本质上是让这些注意力维度都覆盖到。三是把Review从检查变成讨论。我们要求reviewer的每条评论要么是提问要么是建议禁止直接下这样写不行这种结论性表述。作者也不许有抵触情绪收到评论的第一反应不是解释而是先想对方为什么会这样问。这一步很微妙一旦双方进入防御姿态整个Review就废了。后面我会单独写这个部分这里先不展开。另外一个容易被忽略的改变是把自动化工具能做的事情全部交给工具。格式、风格、基础静态检查这类问题机器比人可靠不该浪费人的注意力。人的精力应该集中在机器判断不了的事情上——设计合理性、逻辑正确性、边界和异常路径、长期可维护性。这套做法跑通之后我们团队线上缺陷率肉眼可见地降了下来更意外的是代码质量提升之后新成员上手的速度也快了因为他们可以通过历史MR记录看到每个改动背后的决策过程这比看文档有用得多。2. 审查前要做好的准备模板、范围与提交规范很多人以为Code Review是从reviewer打开diff那一刻开始的其实不是。Review的起点是作者准备提交的那一刻甚至更早。作者在提交之前做的准备工作直接决定了reviewer能不能有效地审查。好的准备能把reviewer的注意力聚焦到该看的地方而不是浪费在猜测意图、推断背景上。2.1 用MR模板引导作者交代背景我们团队一开始没有MR模板大家提交的MR描述五花八门有的写fix bug有的干脆只贴一个需求链接。reviewer每看一个MR都要先去翻需求文档、翻聊天记录搞清楚这个改动的背景和目的精力消耗非常大。后来我们抄了一套简洁的模板字段不多但每个字段都有用途这个MR要解决什么问题改动的核心思路是什么影响范围评估涉及哪些模块、哪些接口、有没有数据库变更自测结果哪些场景测过哪些没测风险点和需要重点review的位置这个模板不是形式主义的产物它的核心价值在于强迫作者在提交前把问题想清楚。很多人在填模板的过程中就会发现自己的改动逻辑有问题或者发现自己想漏了某个场景。我自己就经历过几次写着写着发现我这个改动会不会影响另一个模块的缓存然后默默回去重写。这不就是提前把问题消灭在最便宜的阶段吗另外MR描述里的风险点和需要重点review的位置这个字段非常有用。它相当于作者给reviewer画了一个注意力地图明确告诉对方这里我没把握帮忙重点看看。这比reviewer在几百行diff里盲人摸象高效得多。2.2 小步提交为什么300行是个合理阈值把MR控制在300行以内听起来像是一个拍脑袋的数字但实操下来是有道理的。人脑的工作记忆容量大概能同时处理四到五件事情假设一个reviewer需要理解函数调用链、数据流、异常路径、边界条件面对一个300行的MR他可以在合理时间内建立完整的心智模型。但当diff超过1000行的时候大部分人的处理策略会自动降级为快速扫一遍注意力根本覆盖不过来。控制MR大小的核心手段是控制需求粒度。一个用户故事如果开发起来要一周想拆成300行以内的MR其实很难因为业务逻辑本身的复杂度摆在那里。但对于大多数业务系统来说一个功能迭代完全可以拆成好几个连续的MR第一步先加接口定义和DTO类第二步实现核心逻辑第三步补充边界处理和异常分支第四步接前端或者加持久化。每一步之间的编译状态都是能通过的reviewer可以分次审查逻辑反而更清楚。当然有些改动天生就是大面积重构比如框架升级、目录结构调整这类大MR无论如何都拆不了。这时候我们的做法是降低reviewer的单次负担可以约定一个时间窗口拉一个专门的review会议大家坐在一起过这个大MR而不是让reviewer自己默默消化。后面讲同步评审的时候我会再提到。2.3 提交信息与自动化检查的前置提交信息这事看起来小但直接影响review效率。规范的提交信息应该表达为什么改而不是改了什么。举个反面例子fix bug这种提交信息基本上什么都没说。正面例子是修复用户头像上传后偶现500原因是上传接口未对multipart格式做校验现在在controller层增加格式白名单。这种提交信息本身就带着设计决策reviewer不用猜。另外自动化检查的前置一定要做足不能把格式问题留给reviewer去备注。我们团队在CI里配置了以下检查ESLint/Checkstyle等静态检查工具统一代码风格单元测试和集成测试自动跑覆盖率低于阈值会告警基础的静态安全扫描依赖漏洞扫描CI上有任何一项红掉MR就不能被review。这条规则很硬性从一开始就定死。因为一旦先帮我看一下逻辑格式我回头改这种请求开了头人性就会天然地把越来越多的事情往后拖最后reviewer必然要帮作者擦屁股。我见过最崩溃的一次review是一个前端同事提交了700多行代码CI上eslint报了几十个warning单测还挂了两个。他在群里说warning没关系挂掉的单测是因为本地环境问题然后求大家赶紧review因为这个功能第二天就要上。这种场景下任何流程都顶不住——要么你顶着环境风险合进去要么你把所有人晾在这里花一个下午帮他清理垃圾代码。后来我们把CI的红灯作为MR review的硬前置条件谁来说情都不行这种情况就杜绝了。3. 审查中到底该看什么五个注意力维度很多Review做得浅不是因为参与的人不认真而是因为不知道该看什么。拿着一份diff从哪里入手逐行读吗读完又能怎样我总结了一套自己的注意力分配方法把Review的注意力分成五个维度每个维度有不同的关注点和检查方法实测下来能明显提高review的深度和效率。3.1 逻辑正确性代码是否真的做了它该做的事这个是review的基础维度也是最重要的维度。reviewer拿到diff先不着急逐行看细节而是先看整体流程这个改动完成了一个什么操作输入是什么输出是什么中间状态有哪些然后把注意力放在分支和异常路径上。正常的happy path其实不容易出问题问题往往藏在if条件里。有几个高频bug场景我每次review都会专门留意空指针和空集合上游返回的对象是否可能为null循环里取集合元素时是否确认了非空边界条件循环的起止索引是否在边界情况下出现off-by-one错误日期处理是否考虑了时区和夏令时并发场景共享变量是否有并发修改风险多线程环境下集合是否用了线程安全版本资源释放数据库连接、IO流、HTTP客户端是否在异常路径下也被正确关闭或回收还有一类逻辑问题容易被忽略就是改动是否破坏了原有行为。比如一个方法原来接收null时返回默认值新代码直接NPE了或者一个接口的响应结构变了前端解析时没适配。这类问题需要reviewer对旧逻辑有基本的了解或者至少要有意识去问这个改动会影响哪些调用方。3.2 安全问题不只是注入和越权安全审查听起来像安全团队的活但业务开发的reviewer完全可以提前发现很多低级安全问题。我review时关注的安全点包括输入校验接口是否对request参数做了合法性校验类型、长度、格式、枚举范围是否有恶意构造的绕过可能权限控制这个接口是否做了鉴权操作是否校验了资源归属普通用户是否可能通过修改ID访问他人数据敏感信息日志里是否打印了密码、token、身份证号等敏感字段接口返回值是否泄露了不必要的内部信息表达式注入、路径穿越、反序列化漏洞这类经典问题如果reviewer有安全意识看到可疑代码时搜一下上下文就能发现。我之前在一个老项目里review到一个接口直接把用户传入的文件名拼到了文件路径里然后读取文件像/data/files/ fileNamefileName完全没过滤路径穿越是妥妥的。这种问题如果reviewer只盯着逻辑看是发现不了的但只要有一颗安全弦看到IO操作、拼路径的代码就会本能地警觉。3.3 性能与效率不是吹毛求疵而是避免灾难性能维度的审查我给自己定的原则是不追求微优化但绝对不能放过那些会出大事的硬伤。什么是硬伤典型的有循环里的一次次查询数据库俗称N1问题。一个列表页循环查库数据量一大响应时间立刻爆炸。大量数据全部加载到内存里处理不借助分页或流式处理。死循环或循环条件在极端输入下不满足退出条件。高并发场景下使用了共享可变状态导致性能瓶颈或线程安全问题。大事务里执行了耗时操作比如在事务里调外部API、发消息队列、做大量计算导致数据库连接长时间占用。reviewer看到循环结构时要习惯性地问一句这个循环体内有没有IO操作循环次数可控吗看到查询时问一句这个查询的索引是否覆盖了where条件中的字段看到缓存使用时问一句缓存失效策略是什么有没有缓存击穿、穿透的风险比较良性的效率问题是那种代码能跑但在数据量变大会很痛的情况。这类问题我在review时会在评论里标注优先级建议优化但不阻塞合并。这样作者可以根据当前的业务规模决定是否处理而不是什么问题都一刀切。3.4 可维护性留给未来同事的善意可维护性是Review中最容易被轻视、但对团队长期影响最大的维度。一个功能写出来如果只有原作者能看懂三个月后他自己可能都看不懂了这个代码就是负债不是资产。我在这个维度关注的点包括命名是否表达了意图。一个方法叫processData还是normalizeUserInput读起来的成本完全不同。函数是否做了单一职责。一个方法超过几百行有多个缩进层级往往意味着它做了太多事情。重复代码是否值得抽取。同样的逻辑出现了两三次抽出公共函数能显著降低后续维护成本。魔法数字和魔法字符串是否有命名常量替代。注释是否解释了为什么而不是是什么。注释这里加1是因为要跳过header行比i1有用一万倍。有时候看到了一个风格上不理想但功能正确的实现我不会直接要求改而是会抛出一个问题和作者讨论如果后续要支持多语言这个硬编码的地方怎么处理把问题留给作者自己去思考比直接下指令更容易让对方接受。3.5 测试覆盖测试是否测了行为而不是实现测试审查被我单独列出来是因为它直接关系到整个代码库的长期稳定性。我对MR里新增代码的测试要求有两条第一正常的成功路径要有测试第二至少一个典型的失败路径或边界条件要有测试。再进一步我会看测试断言的质量。有些测试写了跟没写一样比如一个方法返回boolean测试里只断言了“方法不抛异常”这等于什么都验证。好的测试应该断言具体的返回值、状态变化或副作用。还有一个我很在意的点测试是不是在测实现细节。如果一个测试对内部调用了几次某个函数、某个private方法的实现方式做断言那么这个测试就会在重构时变成绊脚石。测试应该约束的是行为而不是实现。举个例子一个购物车接口测试应该断言加入商品后购物车数量和金额正确变化而不是断言addItem内部调用了calculate方法。Review过程中我在两个地方会重点关注测试一是纯函数和工具类因为这些是复用度最高的代码二是带复杂条件分支的代码因为分支覆盖不足就意味着有隐患没被发现。3.6 给评论分级不是每条评论都值得爆改实操中我还坚持一个习惯给每条review评论分级。这个习惯让我后续的跟踪变得高效了很多。我把评论大致分成三类阻断级逻辑有误、安全问题、会导致线上事故的问题。这类评论一旦发出去MR就不能合并必须等到作者修复并重新提交。建议级不影响正确性但应该改进比如可维护性、性能隐患、命名不佳。这类评论建议但不能阻塞合并作者可以选择现在改或后续处理。可选级纯粹的偏好问题比如代码结构的另一种写法、个人风格偏好。这类评论可有可无作者可以忽略。这个分级的意义在于它给了作者一个很清晰的决策框架——什么必须改、什么应该改、什么随意。没有分级的时候作者面对20条评论往往会陷入三个极端要么全盘照收累到死要么选择性忽略真正重要的要么对每条都纠结半天。分级之后大家都能把精力花在刀刃上。4. 让Review高效落地的工具与流程工欲善其事必先利其器。Code Review的工具链不仅承担着展示diff的功能它的交互方式、通知机制、权限模型都在潜移默化地塑造团队的Review行为。选型和配置做得好的团队Review效率天然会高一个台阶。4.1 工具选型主流平台的取舍现在主流的代码审查工具基本集中在几个方向上。GitHub的Pull RequestGitLab的Merge Request以及一些团队自建的Gerrit还有近几年出现的更轻量的审查工具比如Reviewable、Barkeep这些。从我的实际体验出发如果把团队协作体验和工具学习成本一起考虑我更推荐GitHub或GitLab这种代码托管平台自带的MR/PR能力原因有几点它们和代码仓库集成深度最好diff展示、行级评论、多轮回复、讨论状态一目了然。权限模型和CI/CD集成本身就是平台的一部分review流程可以和流水线联动比如CI挂了就不允许合入。团队新成员一般都已经有平台使用经验学习成本几乎为零。生态插件丰富各种Bot能帮你在MR里自动打标签、自动指派reviewer、自动检查标题规范。Gerrit这种工具更强的场景是它严格的分层代码审查模式比较适合对审控有极致要求的团队。但对于大多数中小团队导入Gerrit的学习成本和日常流程摩擦都不小收益没那么明显所以我一般不太推荐。这里额外说一句平台只是载体不要指望换个工具就能让Review变好。工具解决的是信息展示和流程推进的效率真正解决Review质量问题的是人的行为和团队的文化。我见过用最原始的邮件列表做Review也做得非常扎实的开源项目关键还是参与者怎么思考这件事。4.2 在GitLab MR上配置一套自动流程以GitLab为例我把我们团队实际用到的一些基本配置和自动化流程分享出来你可以直接照搬首先在项目里配置Push Rule强制提交信息符合规范。GitLab有原生的提交信息正则校验能力我们用的正则大概长这样^(feat|fix|docs|style|refactor|perf|test|build|ci|chore)\(.\): .{10,200}$这个规则强制提交信息包含类型、影响范围和一个不少于10个字符的描述。前面说的fix bug这种一笔带过的提交在push环节就被拦住了。其次配置Merge Request的批准规则。我们团队的项目至少需要2个approval才能合并其中至少1个必须来自代码Owner也就是维护这个模块的常驻reviewer。数据库迁移相关的MR还需要额外一个DBA角色的approval。这个配置在GitLab的Settings - Merge Requests - Approval rules里设置。第三把自动化检查作为合并流水线的一部分。GitLab CI中我们做了一个简单的并行任务提交后同时跑lint、单元测试、安全扫描和源码静态检测。MR的merge按钮只有在所有流水线通过之后才会变绿任何一环红了都不允许手动合入。stages: - lint - test - security lint-job: stage: lint script: - npm run lint unit-test-job: stage: test script: - npm run test:coverage security-scan: stage: security script: - npm run audit:prod这些自动化不是取代人而是把人从低价值的重复劳动里解放出来。reviewer打开一个MR的时候基本的质量底裤已经由机器兜住了他可以把全部注意力放在真正需要人类智慧的维度上。4.3 异步Review 同步Review双轨并行大多数Review是异步的——作者提交MRreviewer在自己方便的时候打开看然后留言。异步Review效率高、时间自由是主流。但异步Review有一个天然的短板当改动特别大、或者设计层面有分歧时靠留言来来回回效率极低几句话说不清楚还会产生误解。我们团队在异步Review之外推行了一种定时同步Review的机制。每周固定两次每次45分钟拉上所有开发同学对着提前挑选出来的、复杂度较高的MR过一个系统设计相关的Review。这种会议上reviewer不是逐行看代码而是讨论整体设计方案的合理性——数据结构选型是否合适模块边界划分是否清晰未来的扩展空间是否预留同步Review的价值不只是在代码上它还是一个非常高效的团队教学场景。新人在会上听到资深同事怎么分析一个设计的利弊比自己在网上看一百篇文章都管用。而且一旦团队形成了这种公开讨论代码的氛围日常异步Review里的讨论感也会自然地变强大家不再把Review当成单向的审批而习惯性地你一句我一句地交流起来。同步Review要控制次数和时长不然就成了另一种形式的会议负担。我们坚持的原则是同步Review只讨论设计和跨模块影响不逐行读代码只讨论有分歧的问题不重复异步Review已经做完的事情。4.4 行级评论的正确打开方式行级评论是Review的核心交互方式但同样的功能不同人的用法差异非常大。有些人的评论像审问有些人的评论像求助还有一种人会在你不经意间提一句这一行是不是缺了个边界判断一下就点醒了你。差别在哪我把自己的评论习惯总结成几个句式模板效果不错分享出来提问式这里的timeout为什么要设置成5秒是基于什么场景测算的 — 比5秒太短了更容易被接受也真的能帮助作者重新审视参数选择。假设式如果用户在上传的同时点击了取消这个文件句柄还会被正确关闭吗 — 把一个潜在的bug包装成一个具体的场景假设对方一下子就能意识到问题。建议式如果后续要支持多租户这个硬编码的数据库名会不会成为拦路虎要不要考虑抽到配置里 — 这种话术有助于提醒长期利益而不是单纯指摘当前设计。我自己避免的句式包括纯指令式的改成xxx、情绪化的为什么这样写、以及脱离上下文的建议用xxx因为我们之前项目都这么写——最后这种是最没有说服力的因为每个项目都有自己的约束条件。另一个细节是给评论附上信息来源。比如你怀疑某个API在高并发下有性能问题最好贴上官方文档链接或者之前的性能测试数据。有理有据的评论会让作者更容易心服口服地接受也能减少来回辩解的时间。5. 常见问题与排查技巧实录做Review时间长了遇到的坑真不少。有些问题会反复出现特别是在团队从走走形式向认真Review过渡的阶段。这里我挑几个最典型的问题整理一下表现、原因和我们的应对策略希望能帮你少走弯路。5.1 问题Review总是滞后一拖就是三五天这是最普遍的问题。MR提交之后reviewer因为自己任务太多就把Review搁置了等想起来的时候分支已经和主干漂移很远Context信息也丢了看着更烦于是继续拖延最后在合并前突击看一遍。我们试过很多办法最有效的是给Review设置SLA并且用机器人来提醒。GitLab上可以配置一种webhook当MR等待review的时间超过8小时自动在群里发提醒。之后我们把等待时间缩短到4小时刚开始团队觉得被催得很烦但运行两周后所有人都意识到了它带来的好处Review越及时作者接下来要做的代码越少整体返工率降低得非常明显。另一个实操层面很管用的办法是把Review规划进迭代的时间预算里。估算开发任务时预留10%到15%的时间给Review和被Review不要让排期吃得满满的。这个习惯一开始会觉得拖慢了迭代速度但坚持几个月后你会发现漏到测试阶段的bug少了反过来省下的返工时间远超预留的那一点。5.2 问题评论变成人身攻击Review变成战场技术讨论升级成人身攻击这个在业内太常见了。一条评论写得冲一点作者立刻防御心理拉满两个人就开始在MR底部一来一回地辩论最后经常是代码没改好关系还僵了。预防这个问题要从制度和文化两个层面入手。制度层面我们要求所有review评论聚焦代码而非作者严禁使用你你写的东西这种指向性称呼。这条规则看起来有点死板但执行起来效果立竿见影。文化层面更微妙。团队负责人在类似冲突刚冒头的时候要及时介入把讨论拉回技术本身。比如两个人在设计上争执不下负责人可以说这个决策我们现在无法确定谁是对的要不咱们用数据说话两个人分别列一下方案的利弊下次同步review时大家投票。把矛盾转化为任务情绪自然就消解了。5.3 问题浅层Review走过场发现不了真问题另一种常见问题是Review全做完了approval也给了但什么实质性的问题都没发现。你要说完全没效果也不公平毕竟格式问题、命名问题确实被挑出来了一些但真正的逻辑缺陷、安全漏洞一个都没发现。想让Review变深我总结了几条经验控制MR大小。前面讲过的300行阈值这是深度Review的基础。diff一大人脑就投降再好的方法论也发挥不出来。利用checklist。团队沉淀一个review checklist里面列出高频bug类型和常见问题reviewer在开始前先过一遍checklist让大脑进入状态。这个checklist不是形式主义它其实是在帮你校准注意力。让多人分维度review。一个reviewer看逻辑另一个reviewer专门看安全第三个看测试覆盖。分工明细后每个人的工作负担都减轻了但整体覆盖的深度反而增加了。我们团队沉淀出了一份review checklist的初版核心就是几条高频检查项你也能结合自己的技术栈和踩坑历史制定一份边界条件与异常路径是否覆盖输入校验是否充分是否有可能被恶意绕过循环内是否可能存在IO或数据库操作敏感信息是否可能被打印或泄露新增的依赖是否必要版本是否已知存在漏洞测试是否覆盖典型成功路径和至少一个失败路径5.4 问题MR主题失控越改范围越大有一次我们做了一个纯前端小改动reviewer在评论里建议顺手把相关的过渡动画一起调整了吧作者回答说好然后因为动画的调整又牵出了样式重构最后这个MR从200行的改动膨胀成了1500行的重构。本来半天能合入的改动拖了四天。MR范围失控是Review流程中特别隐蔽的成本杀手因为它看起来每个人都在积极干活儿但实际效率极低。我们定了一个规矩MR只做它描述里写的事。任何超出范围的改动不管多小都建议另开一个MR来跟踪。reviewer在评论里发现问题域之外的点可以提出但不要期望在这个MR里解决可以记录到backlog里。规矩定了之后大家执行起来一开始会觉得别扭但慢慢就适应了。拆出去的MR虽然要多走一遍流程但每个MR的review难度和时间都大幅下降整体效率反而更好了。5.5 新手如何快速提升Review能力很多刚入行的同学会觉得让我review别人的代码我不知道说什么这很正常。Review能力不是天生的是通过练习和积累建立起来的。我的建议是分成几步走第一步从跟着看开始。先看资深同事对同一个MR的评论学习他们关注哪些点、用什么方式提问。初始阶段不用追求独立输出观点看懂也是一种能力。第二步带着checklist走流程。每次review之前把checklist过一遍逐项检查自己看过的代码里有没有对应的情况。这时候你是依赖清单来发现问题的不需要完全靠直觉慢慢地清单就变成了你的思维习惯。第三步给自己下找茬指标。比如每次review至少要提出一个有价值的问题不一定是bug可以是对某个设计决策的疑问。给自己定一个数量指标会强迫自己看得更深入。第四步从自己熟悉的模块开始。对业务逻辑熟就比较容易看出逻辑问题对数据库模型熟就更容易发现数据一致性的隐患。不要上来就去review自己不熟悉领域的代码挫败感太强而且确实也发现不了什么有价值的东西。写在最后这两年我越来越觉得代码审查这块内容最难的其实不是技巧和方法而是让团队打心底里认可认真Review对自己有好处这个基本盘。reviewer认真看别人的代码能学到不同人的设计思路和实现技巧这本身就是一种极其高效的学习作者被认真审查改了问题之后线上少出事故省下来的是自己半夜起来救火的痛苦。双赢的事情为什么大家还是不愿意做因为人都会倾向于做那些反馈周期短的快事而Review的反馈周期实在太长了——你今天认真看了一个MR不会获得任何即时的正反馈要等几个月后线上少了一个故障你才隐隐约约感到好像有点用。所以团队在推行Code Review时除了方法和工具之外一定要花精力营造正反馈。比如每月公开表扬那些提出了关键问题的review评论把它们当作团队的学习材料或者统计上线后有缺陷的MR与没有缺陷的MR的review深度差异用数据让大家信服。这些动作做多了团队会慢慢从被逼着review转变到从review中得到成长。最后再分享一个小技巧。我以前review代码看一半经常觉得逻辑绕不过来就想放弃后来我养成了一个习惯在review的时候开一个本地终端把关键的数据流、变量状态随手画一下、写一下相当于在纸上模拟代码的执行过程。这个方法看起来有点笨但真的能帮你抓住很多微妙的逻辑问题比反复用眼睛看代码高效得多。这个习惯我用了好几年推荐给所有被复杂MR困扰过的朋友试试。