代码审查、重构与提交前检查：小项目开发流程实战指南

1. 为什么代码审查、重构和提交前检查要放在一起做很多人把代码审查、重构和提交前检查当成三件独立的事审查是审查重构是重构提交前检查就是跑一下测试。我一开始也这么想直到在一个小项目上连续踩了几次坑才发现这三件事本质上是一条流水线上的三个工位拆开做就会出问题。先说一个我亲身经历的教训。当时我在做一个数据同步的小工具功能不复杂大概两千行代码。写完第一版之后我觉得代码有点乱就花了一个下午做重构把几个大类拆成了小模块方法名也改得更清晰了。重构完之后我跑了一下主流程没问题就提交了。结果第二天同事拉下代码一跑发现配置文件读取的路径变了因为我在重构的时候顺手把配置加载的逻辑挪到了另一个模块里但忘了同步更新默认路径的拼接方式。主流程能跑通是因为我本地有一个旧的缓存文件而同事那边是全新环境直接就报错了。这件事让我意识到一个问题重构本身没有错错的是我在重构之后没有做完整的提交前检查。而如果当时有代码审查环节同事在看我改动的时候大概率会发现路径拼接的变化因为审查的时候人会关注“改了什么”而不是“能不能跑”。所以这三件事的关系是这样的代码审查是让别人帮你发现你看不到的问题重构是让代码变得更容易被审查和维护提交前检查是最后一道防线确保你的改动不会破坏别人的环境。三者缺一不可而且顺序很重要。还有一个常见的误区是觉得小项目不需要这些流程。恰恰相反小项目往往是一个人或者两三个人在维护没有专门的测试团队没有CI/CD流水线一旦出问题就是直接影响到实际使用。大项目有完善的工具链兜底小项目只能靠自己的习惯和流程来兜底。这篇文章我会围绕一个模拟的小项目场景把代码审查、重构和提交前检查这三个环节拆开来讲每个环节都会说清楚“为什么这么做”“具体怎么做”“我踩过哪些坑”。适合有一定编程基础、正在独立开发或者在小团队里协作的开发者参考。不管你是写Python、JavaScript还是Java思路是通用的具体工具可以根据自己的技术栈替换。2. 代码审查小项目里怎么做才不流于形式2.1 审查的时机比审查的内容更重要很多人觉得代码审查就是写完代码之后找人看一眼但其实审查的时机决定了审查的效果。我试过两种方式一种是写完整个功能之后再审查另一种是每完成一个小模块就审查一次。实测下来后者的效果要好得多。原因很简单当你写完整个功能再审查的时候改动可能涉及十几个文件、上千行代码审查的人看到这么大的diff心理上就已经疲惫了很容易只扫一眼就点通过。而且这个时候如果发现设计上的问题改动的成本已经很高了因为很多代码已经基于错误的设计写完了。小模块审查的好处是每次改动控制在两三百行以内审查的人有精力逐行看而且发现问题的时候改动成本低。我一般的做法是一个功能拆成三到四个小模块每个模块完成后就提交一次审查请求审查通过后再继续下一个模块。具体操作上如果你用的是Git可以这样做# 创建一个功能分支 git checkout -b feature/data-sync # 完成第一个小模块后提交 git add module_a.py git commit -m feat: 实现数据读取模块 # 推送并创建审查请求 git push origin feature/data-sync然后在代码托管平台上创建一个审查请求指定审查人。审查人看完之后如果有意见就评论你修改后再推送直到审查通过。注意不要在一个审查请求里混合多个不相关的改动。比如你既改了数据读取逻辑又改了日志格式还顺手升级了一个依赖这三件事应该分成三个审查请求。混在一起会让审查人很难判断哪些改动是相关的哪些是顺带的。2.2 审查清单小项目里最值得关注的五个点大公司通常有很详细的代码审查清单几十条规则但对于小项目来说逐条对照根本不现实。我根据自己的经验总结了一个精简版清单只有五个点但覆盖了大部分常见问题。第一看接口设计是否合理。这是最重要的一点。接口包括函数签名、类的方法、模块之间的调用方式。审查的时候先看接口因为接口一旦定下来后面所有代码都要围绕它来写改起来成本最高。具体看什么呢看参数是不是太多了超过四个就要考虑封装成对象看返回值是不是清晰不要返回一个含义不明的字典看方法名是不是能准确表达意图。第二看边界条件有没有处理。这是小项目最容易出问题的地方。比如读取文件的时候文件不存在怎么办网络请求超时怎么办输入参数为空怎么办。审查的时候重点看这些地方有没有做判断判断的逻辑对不对。第三看有没有硬编码。小项目里硬编码特别常见因为方便。但硬编码的路径、端口号、密钥一旦需要修改就会很麻烦。审查的时候看到硬编码就要提出来至少改成配置项。第四看错误处理是否合理。不是说要每个地方都try-catch而是说错误发生的时候程序的行为是不是可预期的。比如一个函数出错的时候是返回None、抛异常还是返回错误码整个项目应该统一。第五看有没有明显的性能问题。小项目一般性能要求不高但有些写法会埋下隐患。比如在循环里反复读文件、反复建立数据库连接、反复做重复计算。这些在数据量小的时候看不出来数据量一大就出问题。这五个点看起来简单但实际审查的时候能全部覆盖到就已经很不错了。我自己的习惯是在审查请求的描述里把这五个点列出来审查人逐条确认这样不容易遗漏。2.3 审查意见怎么提才不伤和气又有效代码审查有一个很微妙的点怎么提意见。提得太直接对方可能觉得你在挑刺提得太委婉对方可能get不到你的意思。我试过几种方式最后总结出一个原则对事不对人说清楚为什么。举个例子你看到一段代码是这样的def get_user_data(user_id): conn sqlite3.connect(data.db) cursor conn.cursor() cursor.execute(fSELECT * FROM users WHERE id {user_id}) return cursor.fetchone()如果你只说“这里有问题”对方可能不知道你说的是什么问题。如果你说“这里应该用参数化查询”对方可能知道要改但不知道为什么要改。比较好的提法是这里用f-string拼接SQL会有注入风险如果user_id是外部传入的攻击者可以构造恶意输入。建议改成参数化查询cursor.execute(SELECT * FROM users WHERE id ?, (user_id,))。另外连接用完记得关闭或者用with语句管理。这样说既指出了问题又解释了原因还给出了修改建议。对方看到之后不仅知道要改还知道为什么要改下次写代码的时候就会注意。还有一个技巧是把“你”换成“这里”或者“这段代码”。不要说“你没有处理异常”而要说“这段代码没有处理异常”。虽然意思一样但后者听起来更像是在讨论代码本身而不是在指责写代码的人。2.4 审查通过之后别急着合并审查通过之后很多人就直接合并到主分支了。我建议再等一步让审查人确认一下你的修改。因为审查意见提出来之后你修改了代码但审查人可能没有再看一遍。如果修改的时候引入了新的问题直接合并就会把问题带到主分支。正确的做法是修改完之后再推送一次在审查请求里回复“已修改请再看一下”。审查人确认没问题之后再合并。这一步多花几分钟但能避免很多返工。3. 重构什么时候该动手什么时候该忍住3.1 重构的信号这五种情况说明代码该整理了重构不是什么时候都能做的也不是什么时候都需要做的。我总结了一下出现下面这五种情况的时候重构的收益比较大。第一种同一个逻辑在三个以上的地方出现。比如你发现有三个函数都在做类似的数据校验那就可以考虑抽成一个公共函数。注意是三个以上两个地方重复可能只是巧合三个以上就说明确实有共性的需求。第二种一个函数超过五十行。五十行不是绝对标准但超过这个数通常意味着这个函数做了太多事情。我一般的做法是看这个函数能不能拆成几个步骤每个步骤一个子函数。第三种修改一个功能需要改五个以上的文件。这说明模块之间的耦合太紧了改一个地方会牵连很多其他地方。这时候需要考虑重新划分模块边界。第四种加一个新功能需要改很多旧代码。这说明原来的设计没有预留扩展点。比如你原来只支持一种数据格式现在要加一种新格式如果发现要改十几个地方那就说明需要抽象出一个接口。第五种你自己看代码的时候觉得费劲。这是一个很主观但很准的信号。如果你自己写的代码过了一个月再看需要花很长时间才能理解那说明代码的可读性有问题值得重构。反过来下面这些情况我建议先忍住不重构代码能正常工作且最近不会改动、项目马上要交付没有时间测试、你还不完全理解这段代码的逻辑。重构的前提是你对代码的行为有充分的了解否则很容易改出问题。3.2 小步重构的具体手法从改名开始重构最怕的就是一次改太多改完之后出了问题不知道是哪里改坏的。我的经验是每次只做一种类型的重构做完之后跑测试通过之后再继续下一种。最安全的重构是改名。把含义不清的变量名、函数名改成能表达意图的名字。比如把d改成user_data把process改成validate_and_save。改名不会改变代码的行为但能大幅提升可读性。# 改名前 def proc(d): if d[t] 1: return d[v] * 2 return d[v] # 改名后 def calculate_discounted_price(product): if product[type] premium: return product[value] * 2 return product[value]改名的技巧是名字要说明“是什么”而不是“怎么做”。比如calculate_discounted_price说的是这个函数做什么而multiply_by_two说的是怎么做。前者更好因为如果以后折扣逻辑变了函数名不需要改。第二种安全的重构是提取函数。把一段代码从大函数里抽出来变成一个独立的小函数。这样做的好处是原来的大函数变短了抽出来的小函数可以单独测试。# 重构前 def process_order(order): # 校验 if not order.get(items): raise ValueError(订单不能为空) if order.get(total, 0) 0: raise ValueError(金额必须大于零) # 保存 db.save(order) # 通知 send_email(order[user_email], 订单已创建) # 重构后 def validate_order(order): if not order.get(items): raise ValueError(订单不能为空) if order.get(total, 0) 0: raise ValueError(金额必须大于零) def process_order(order): validate_order(order) db.save(order) send_email(order[user_email], 订单已创建)提取函数的时候有一个判断标准如果一段代码可以用一句话描述它在做什么那它就可以被提取成一个函数。比如上面那段校验代码一句话就是“校验订单”所以提取成validate_order。第三种重构是消除重复。当你发现同样的代码出现在多个地方就把它抽成一个公共函数或者基类。但要注意消除重复的前提是这些代码真的是同一个逻辑而不是碰巧长得像。如果两个地方的代码只是看起来一样但实际含义不同强行合并反而会导致以后改一个地方影响另一个地方。3.3 重构之后必须做的验证别只跑主流程重构之后要验证这个大家都知道。但很多人只跑一下主流程觉得没问题就完了。我踩过的坑告诉我重构之后的验证要覆盖边界条件和异常路径。具体怎么做呢我一般分三步第一步跑一遍现有的单元测试。如果项目没有单元测试那至少写几个针对被重构代码的测试。不用追求覆盖率但要把主要的输入输出组合覆盖到。第二步手动测试边界条件。比如空输入、超大输入、特殊字符、并发调用。这些情况在主流程里可能不会出现但实际使用中一定会遇到。第三步对比重构前后的行为。如果条件允许可以在重构前记录一些典型输入对应的输出重构后再跑一遍看结果是否一致。这个方法在重构复杂逻辑的时候特别有用。注意重构之后如果发现行为变了先不要急着改代码先确认一下原来的行为是不是正确的。有时候重构会暴露出原来的bug这时候应该先修复bug而不是让重构后的代码去兼容错误的行为。3.4 重构和审查的配合先审查再重构还是反过来这个问题我纠结过很久。后来我的做法是小重构直接做做完之后一起审查大重构先审查方案通过之后再动手。小重构比如改名、提取函数、消除明显的重复这些改动不会影响外部行为直接做完之后在审查请求里说明“这是重构没有改变行为”审查人重点看改动是否真的没有改变行为就行。大重构比如重新划分模块、更换设计模式、调整接口这些改动会影响多个文件甚至整个项目的结构。这种重构应该先写一个简单的方案说明包括为什么要重构、打算怎么改、影响范围有多大让审查人先看方案。方案通过之后再动手避免改到一半发现方向不对。我见过一个反面的例子有个开发者觉得项目的数据库访问层设计得不好花了一周时间重写了一遍提交审查的时候审查人问“为什么要重写”他说“原来的不好”。审查人又问“哪里不好”他说“不够优雅”。这种重构就是没有明确目标的改完之后除了代码风格变了其他方面没有任何提升反而引入了新的bug。4. 提交前检查最后一道防线怎么设4.1 自动化检查让工具做它能做的事提交前检查的第一层是自动化检查。这部分工作应该交给工具不要靠人。我一般会配置三个检查代码格式检查、静态类型检查、单元测试。代码格式检查用来自动统一代码风格。Python用blackJavaScript用prettierJava用checkstyle。这些工具能自动格式化代码避免因为空格、换行、引号风格这些小事在审查的时候浪费时间。# Python项目配置pre-commit # .pre-commit-config.yaml repos: - repo: https://github.com/psf/black rev: 23.1.0 hooks: - id: black - repo: https://github.com/pycqa/flake8 rev: 6.0.0 hooks: - id: flake8配置好之后每次git commit的时候会自动运行这些检查不通过就不让提交。这样能保证进入仓库的代码风格是一致的。静态类型检查用mypyPython或者TypeScriptJavaScript。类型检查能在编译阶段发现很多潜在的错误比如传错了参数类型、调用了不存在的方法。小项目里很多人觉得类型检查麻烦但实际用下来它能帮你省下很多调试的时间。单元测试是最后一道自动化防线。我建议至少覆盖核心逻辑和边界条件。不需要追求100%的覆盖率但关键路径一定要有测试。测试写完不是就完了每次提交前都要跑一遍确保没有破坏已有的功能。# 提交前手动跑一遍测试 pytest tests/ -v # 或者配置成pre-push钩子 # .git/hooks/pre-push #!/bin/sh pytest tests/ || exit 14.2 手动检查清单那些工具查不到的东西自动化工具能查语法错误、格式问题、类型不匹配但有些东西工具查不到必须手动检查。我总结了一个提交前的手动检查清单每次提交前花两分钟过一遍。第一检查有没有调试代码残留。比如print语句、console.log、断点、注释掉的代码块。这些东西在开发的时候有用但提交上去就是垃圾。第二检查配置文件有没有改错。比如数据库地址是不是从测试环境改回了生产环境日志级别是不是从DEBUG改回了INFO超时时间是不是调回了正常值。第三检查依赖有没有更新。如果你在开发过程中安装了新的依赖确认一下依赖文件requirements.txt、package.json有没有同步更新。反过来如果你删除了某个依赖的使用确认一下依赖文件里有没有把它移除。第四检查提交信息是否清晰。提交信息要能说明这次提交做了什么。我一般的格式是类型: 简短描述比如feat: 添加数据导出功能、fix: 修复空指针异常、refactor: 重构用户模块。类型包括feat、fix、refactor、docs、test、chore等。第五检查有没有遗漏的文件。有时候新建了文件但忘了git add提交上去之后别人拉下来发现少文件。用git status确认一下所有该提交的文件都提交了。4.3 提交信息的写法三个月后的你能看懂吗提交信息看起来是小事但其实很重要。我给自己定了一个标准三个月后回头看这条提交信息能不能想起来当时改了什么、为什么改。不好的提交信息长这样update fix bug 修改好的提交信息长这样fix: 修复用户名为空时登录崩溃的问题 当用户名为空字符串时登录接口会抛出KeyError。 原因是查询用户时没有做空值判断直接取了字典的key。 现在在查询前增加了空值校验返回明确的错误提示。好的提交信息包含三个部分做了什么fix、改了什么空值校验、为什么改避免崩溃。这样即使过了很久回头看也能快速理解这次提交的意图。如果一次提交涉及多个不相关的改动建议拆成多次提交。比如你既修复了一个bug又添加了一个新功能应该分成两次提交。这样以后如果要回滚其中一个改动不会影响到另一个。4.4 提交前的最后一步在干净环境里验证这是我最想强调的一点。很多问题在开发环境里发现不了因为开发环境里有很多缓存、临时文件、本地配置。只有在干净环境里才能暴露出来。我一般的做法是提交前在一个全新的目录里克隆一份代码按照README的说明从头配置一遍跑一遍主流程。这个过程能发现很多问题比如依赖没有写全别人拉下来装不上配置文件没有提供模板别人不知道要配哪些项数据库初始化脚本没有更新新环境跑不起来文档里的步骤过时了和实际代码不一致这个过程大概花五到十分钟但能避免很多“在我机器上能跑”的尴尬。提示如果项目有CI/CD流水线这一步可以交给流水线自动做。每次推送代码后流水线在一个干净的环境里自动构建和测试结果会通知你。小项目如果没有流水线手动做这一步也值得。5. 三个环节串起来一个完整的实操流程5.1 从写代码到合并的完整时间线把上面说的三个环节串起来一个完整的流程大概是这样的第一步写代码之前先拉一个功能分支。不要在主干上直接开发这样即使出了问题也不会影响其他人。第二步每完成一个小模块就提交一次。提交信息写清楚这个模块做了什么。提交之后推送分支。第三步创建审查请求。在审查请求的描述里说明这次改动的内容、影响范围、需要重点看的地方。指定审查人。第四步根据审查意见修改。修改完之后再推送回复审查人请对方确认。第五步审查通过后做重构。如果审查过程中发现了设计上的问题先重构再合并。重构之后重新跑测试。第六步提交前检查。跑自动化检查、过手动清单、在干净环境里验证。第七步合并到主干。合并之后删除功能分支。这个流程看起来步骤很多但实际做起来每个步骤花的时间并不多。小模块的审查可能就十分钟重构可能就半小时提交前检查可能就五分钟。加起来可能比出问题之后调试的时间还短。5.2 工具链推荐小项目够用就好工具不需要多够用就行。我推荐一套最小化的工具组合用途推荐工具说明版本控制Git基础工具必须代码托管任意支持审查请求的平台小项目用免费的就行代码格式化black / prettier自动统一风格静态检查flake8 / eslint发现潜在问题类型检查mypy / TypeScript可选但推荐单元测试pytest / jest覆盖核心逻辑提交钩子pre-commit自动运行检查这套工具组合的学习成本不高配置一次之后就能长期使用。我建议先从代码格式化和单元测试开始这两个的收益最明显。静态检查和类型检查可以后面再加。5.3 常见问题与应对流程执行不下去怎么办问题一一个人开发没人审查怎么办可以自己审查自己。具体做法是写完代码之后隔一天再看或者换一个环境比如换个编辑器再看。隔一段时间之后你看自己的代码会像看别人的代码一样更容易发现问题。问题二项目太急没时间走完整流程怎么办可以裁剪流程但不能完全跳过。最精简的版本是提交前跑一遍测试提交信息写清楚。审查和重构可以等有时间的时候补做。问题三重构之后测试跑不过怎么办先确认测试本身是不是正确的。如果测试是正确的那说明重构改变了行为需要修复。如果测试本身有问题先修测试再继续重构。问题四审查意见太多改不过来怎么办把审查意见分类必须改的bug、安全问题、建议改的代码风格、命名、可以不改的个人偏好。先改必须改的建议改的看时间可以不改的说明理由。问题五提交前检查发现的问题太多怎么办说明平时的开发习惯需要调整。把检查中发现的问题记下来下次写代码的时候注意避免。坚持一段时间之后提交前检查发现的问题会越来越少。6. 我踩过的坑和总结的经验6.1 那些年我在提交前检查上偷过的懒我印象最深的一次是提交前没有在干净环境里验证。当时我本地开发环境里有一个环境变量文件里面配置了数据库连接信息。我写完代码之后直接提交了忘了把这个文件加到.gitignore里。结果同事拉下代码之后他的环境变量文件被我的覆盖了连不上数据库排查了半天才发现是这个问题。还有一次是提交前没有跑测试。我觉得改动很小就改了一个函数的返回值类型从int改成了float。本地跑了一下主流程没问题就提交了。结果另一个模块里有个地方对这个返回值做了整除运算float不支持整除直接报错。如果当时跑一遍测试这个问题就能提前发现。这些坑让我养成了一个习惯不管改动多小提交前必须跑测试和在干净环境里验证。这两步花的时间不多但能避免很多低级错误。6.2 重构时最容易犯的三个错误第一个错误是重构和功能修改混在一起。比如你在重构一个函数的同时顺手加了一个新功能。这样如果出了问题你分不清是重构导致的还是新功能导致的。正确的做法是分开提交先重构验证通过后再加新功能。第二个错误是重构没有测试覆盖。如果你要重构的代码没有测试那重构的风险就很高。我的做法是先给要重构的代码补测试确保测试能覆盖主要行为然后再重构。重构之后跑测试通过就说明行为没有改变。第三个错误是重构范围太大。一次重构改了几十个文件改完之后自己都记不清改了哪些地方。正确的做法是小步走每次只改一个类型的问题改完验证再改下一个。6.3 代码审查中最容易忽略的细节代码审查的时候大家通常关注逻辑对不对、有没有bug但有些细节容易被忽略。一个是命名的一致性。比如项目里有的地方用get_user有的地方用fetch_user有的地方用query_user。虽然功能一样但看起来不统一。审查的时候应该提出来统一成一种命名风格。一个是注释的准确性。代码改了但注释没改这种情况很常见。审查的时候看到注释和代码不一致一定要提出来。错误的注释比没有注释更糟糕因为它会误导人。一个是异常处理的完整性。比如一个函数声明了会抛出某种异常但调用方没有处理。或者一个地方捕获了异常但没有做任何处理只是pass了。这些在审查的时候都要关注。6.4 让这套流程真正跑起来的建议最后分享几个让这套流程真正落地的建议。第一从最小的流程开始。不要一上来就搞一套完整的CI/CD先从提交前跑测试开始。等这个习惯养成了再加代码格式化再加静态检查一步一步来。第二把检查自动化。能自动做的就不要手动做。配置pre-commit钩子让工具在提交的时候自动运行检查。这样你不需要记住每次都要跑什么命令工具会帮你记住。第三定期回顾。每隔一段时间回顾一下最近提交的代码看看有没有反复出现的问题。如果有就针对性地调整流程或者补充检查项。第四不要追求完美。这套流程的目的是减少问题不是杜绝问题。总会有漏网之鱼这很正常。重要的是持续改进让问题越来越少。我在实际使用这套流程之后最明显的感受是调试的时间变少了写代码的时间变多了。以前经常花半天时间排查一个提交后才发现的问题现在这些问题在提交前就被拦住了。虽然提交前多花了几分钟但省下的调试时间远远超过这几分钟。