提交前代码质量兜底：审查、重构与自动检查实战

这个系列做到第 4 篇代码片段管理器这个小项目已经完成了核心功能可以添加片段、按标签搜索、通过命令行直接输出到剪贴板整个跑起来也像模像样了。但正因为功能都通了我才意识到一个更隐蔽的问题——代码在能用和好改之间差着一整条质量线。我上一轮提交就吃了大亏。一个搜索函数因为重构时改了参数名但没有同步调用方功能在自己机器上没问题换新电脑一跑就崩。当时我甚至没意识到问题是哪来的因为提交前我只做了一件事跑了一下主流程发现看起来正常就推上去了。后来回头查才明白代码审查、重构、提交前检查这三道工序在我这种小项目里被严重低估了。这篇实战文章不像前面几篇那样讲功能开发而是完整记录我如何在提交前给这个小项目做质量兜底怎么审自己的代码、怎么在保持功能不变的前提下做小步重构、怎么搭一套提交前检查链让低级错误进不了仓库。这可能是这个系列里不花哨但最值钱的一篇。1. 先从一次倒车事故说起为什么三道工序缺一不可1.1 事故现场一次看似无害的提交先说那次翻车。项目的搜索函数最初长这样def search_snippets(queries): results [] for item in snippet_list: for q in queries: if q.lower() in item[content].lower(): results.append(item) break return results我在一次顺手优化里把queries改成了单个字符串query并且把变量item改成了snippet。调用方、测试我都同步改了自测也通过了。看起来天衣无缝对吧问题出在一个藏在深处的调用点那是第三篇里写的一个stats.py模块统计片段数量和常用标签用的它内部也调用了search_snippets(search_queries)。我当时根本没想起来这个模块还依赖这个函数最后也没有跑全量测试。结果就是推上去之后新环境一执行统计功能直接TypeError: search_snippets() got an unexpected keyword argument search_queries。机器不会说谎它只是很耐心地等你撞上它。那次之后我把流程定死了一个提交要进仓库必须走完三条线——审查、重构、检查。审查负责找出不该这样写的地方重构负责把该改的结构改对检查负责确认旧的依赖没有被破坏。三条线可以穿插但任何一条都不能省。1.2 小项目为什么更容易欠技术债大项目有团队、有流程、有 Code Review 机制反而是个人项目和小团队项目最容易积攒烂账原因很朴素一个人写代码没有人会质疑你的命名、你的函数边界、你的异常处理因为我自己看得懂。小项目迭代快功能优先级永远高于整洁度先跑通再说成了默认选项。没有 CI/CD 的环境提交前检查全靠自觉而人的自觉是最不可靠的东西。而且小项目有一个特性代码量增长是指数级混乱的。前 1000 行很清晰2000 行开始有复制粘贴3000 行时你已经不知道哪些函数还被谁引用着。越是这种时候提交前把三道工序走一遍的收益就越大——它不是洁癖是降低后续每次改动的认知负担。这三道工序的关系我可以打个比方代码审查像体检重构像治疗提交前检查像出院前复查。只体检不治疗查出问题也没用只治疗不复查你不知道出院后会不会复发不体检直接治疗你甚至不知道哪里有问题。2. 代码审查不靠肉眼硬扛小项目也能跑起来的分层审查流程2.1 审查的粒度别攒一个月的代码一起看很多人对代码审查有误解觉得那是团队协作才有的东西自己一个人写项目审什么我一开始也这么想直到翻车。后来我把审查这件事简化成了一套自己也能执行的分层流程。核心原则是审查要以提交为单位不要以项目为单位。一个提交就是一个逻辑变更单元你可能改了一个功能、修了一个 bug、调了一段结构。把这个变更的 diff 拉出来看上下文可控思维负担小能真正看进去。我现在的习惯是功能开发的时候每完成一个可独立验证的步骤就git diff看一下这次改动。这时候发现问题成本最低因为代码还在脑子里热乎着。提交消息写完之后再回头看一眼git diff --stat确认这次提交到底动了哪些文件。如果发现自己本来只想改两行结果 diff 里有五个文件那就是改动范围失控了。如果项目有搭档请对方做异步 review没有搭档就自己扮演陌生人角色假装从没看过这代码。2.2 分层审查清单正确性、健壮性、可读性、性能、安全我审查时不是漫无目的地看而是按五个维度过一遍。这五个维度来自我多年踩坑的经验几乎覆盖了我会犯的所有错误类型维度审查目标典型问题正确性逻辑是否符合预期边界是否处理空列表、None、单元素、首尾元素、并发冲突健壮性输入异常、依赖异常是否兜底参数非法、文件不存在、网络超时、权限不足可读性命名是否表意结构是否清晰变量名含义不明、函数超过一屏、注释误导性能复杂度是否符合数据规模循环套循环、N1 查询、重复计算安全是否存在注入、信息泄露命令拼接、SQL 拼接、日志打印敏感信息正确性排第一位因为它直接决定功能能不能跑对。边界是重灾区尤其是列表为空、只有一个元素、内容包含特殊字符这些情况。健壮性看的是外面来的数据不可信这个原则有没有贯彻——用户输入、文件内容、网络响应全都要当恶意或损坏数据来处理。可读性容易被忽视但它是技术债的主要来源。代码不是写给自己看的是写给下次改它的人看的——哪怕那个人就是你本人。性能和安全在小项目里往往不紧急但审查时顺手扫一眼没有坏处一旦项目突然火了这些点会瞬间变成大问题。2.3 实战演示给 search_snippets 做一次审查记录我拿项目里的代码演示一遍审查过程。就是前面那个搜索函数经过功能迭代它已经变成了这样def search_snippets(query, tagNone): if query: query query.lower() result [] for snip in sniplist: flag False if query in snip[content].lower(): flag True if tag and tag in snip[tags]: flag True if flag: result.append(snip) return result乍一看能跑但逐行走查我发现了问题清单命名不清query初看是查询词但后面要求在标题里搜索语义已经扩大变量应该叫keyword或search_text。状态标志冗余flag False这种写法本质上是通过变量缓存一个判断结果完全可以用if ... or ...:直接表达现在多了一个中间变量和两个赋值语句读完还要推理。逻辑可能不符预期当传入queryNone且tag有值时搜索会返回所有带该标签的片段这个行为也许是想要的但没有任何注释说明默认行为未经验证。函数名与实际行为不符名字叫search_snippets但实际筛选条件是或关系这到底是搜索还是过滤命名和语义的偏差会误导调用者。缺少输入校验tag如果传了整数会怎样query如果传了非字符串的迭代对象会怎样直接AttributeError不兜底就等着线上出事故。无文档注释没有 docstring调用方只能靠猜——tag是单个标签还是标签列表传入格式是什么全靠读实现代码。拼写错误sniplist这个变量名我自己看着都觉得别扭。返回类型不稳定result是列表但很多调用方可能希望拿到生成器当前实现会一直累积内存——数据量小没关系量大了就是隐患。这份记录我直接写在提交前的审查单里。每次提交时拉一份这样的清单比在头脑里过一遍可靠得多。因为写下来的东西会被看见看见了就不好意思装看不见。3. 重构的精髓是行为不变从一次重复代码清理看小步改造3.1 重构不是重写先分清两者的边界很多人一提重构就开始大刀阔斧地重写这其实是最危险的路径。重构的定义是在不改变外部行为的前提下调整内部结构。外部行为是什么是函数的输入输出、接口的语义、以及可观察的结果。只要这些不变你改多狠都叫重构一旦变了那叫功能变更需要单独走测试和 review。我给自己立了一条规矩一个提交里不混两种东西。要么是纯结构重构要么是纯功能新增/修复。如果混在一起出了 bug 你根本不知道是重构引入的还是新功能引入的排查成本直接翻倍。另一个原则是小步走。每次只改一个点改完就跑测试确认没破坏再改下一个点。这一步看起来慢实际是大项目最快的路径——因为每一步都有验证每一步都安全你可以在任意位置停下而不是憋一个大改动憋到一半墙倒屋塌。3.2 三个最常见的重构手法提取、合并、命名具体到小项目代码最常用的重构手法就三种提取函数。一段代码里如果出现了一段相对独立、可以语义化的逻辑就把它抽出来变成单独函数。比如把判断一个片段是否命中查询的逻辑从循环里抽出来将来改筛选条件时只改一个地方。合并重复分支。多个if分支结尾做了同样的操作可以合并条件。前面那个search_snippets里的flag逻辑就是典型例子两个判断都只是给一个结果追加失败完全可以直接用if A or B。改命名。命名是重构里成本最低、收益最高的操作。一个准确的名字节省的是每次阅读代码时的理解时间一个误导的名字消耗的则是你未来的时间。项目的sniplist改名成snippetsquery改成keyword一次改完不回头。3.3 从 flag 逻辑看一次完整的小步重构我用项目里那个搜索函数做一次完整重构带你看具体的重构顺序和每一步的验证方式。第一步先给函数加上行为用例。这是重构的前置条件——没有测试保护的重构就是蹦极不系绳。我写了一个简单的测试文件from snippet_store import search_snippets def test_search_hits_content(): assert len(search_snippets(python)) 1 def test_search_respects_tag_filter(): result search_snippets(, tagcli) assert all(cli in s[tags] for s in result) def test_search_empty_query_returns_empty(): assert search_snippets() []注意第三个用例search_snippets()当前返回什么运行时发现函数对空字符串查询会返回全部片段因为if query:对空字符串是假跳过匹配但下面tag为空也跳过最终所有片段都会进入result。这是个隐藏 bug正好被测试用例抓出来了。重构的第一步其实是把行为固定在测试里然后你才知道你改的东西到底有没有改变行为。第二步消除n_snips里的 flag。原代码改为def search_snippets(keyword, tagNone): 按关键词或标签筛选片段。当 keyword 为空且未指定 tag 时返回空列表。 if not keyword and tag is None: return [] keyword (keyword or ).lower() matches [] for snippet in snippets: content_hit keyword and keyword in snippet[content].lower() tag_hit tag is not None and tag in snippet[tags] if content_hit or tag_hit: matches.append(snippet) return matches第三步把判断逻辑提取成独立函数便于将来扩展成标题优先模糊匹配def _snippet_matches(snippet, keyword, tag): if keyword and keyword not in snippet[content].lower(): return False if tag is not None and tag not in snippet[tags]: return False return True重构到这里行为被测试用例约束着每改完一步就跑一遍pytest绿了就继续走。整个过程不超过半小时但收获是原来那个带着隐藏 bug、命名混乱、分支冗长的函数变成了有 docstring、有单元测试、可扩展的结构。这才是重构该有的样子。3.4 重构过程中我最常踩的坑第一个坑是顺手改风格。重构时看到某行不符合格式化规范就顺手改了结果git diff里混进大量非逻辑变动的格式化改动review 的人会无从看起。正确做法是先把格式化交给工具统一跑不要在重构提交里混人肉格式化。第二个坑是重构到一半想加功能。看到这个函数支持了精确匹配觉得不如顺便支持模糊匹配吧——停功能变更请另外开提交否则测试挂了你会怀疑人生。第三个坑是相信自己的手改不会错。重构后觉得逻辑一样只是改了下变量名就不跑测试直接用主流程点一下验证。这个点一下通常只能测到 happy path替代不了自动化测试。我现在每重构一步必跑一次全量测试十步重构跑十次总共可能也就两分钟但心里踏实。4. 提交前检查不是走形式一整套可落地的兜底工具链4.1 人工审查的盲区用工具自动兜住审查依赖人的注意力和经验而人的注意力就是不稳定资源。同一份代码状态好的周日晚上你能看出五个问题赶需求的周五下午你只想赶紧提交。这时候就需要工具来兜底——它们不聪明但胜在无情不会累、不会想偷懒、每次都能稳稳地执行。我把提交前检查分成四个层次层次工具/手段解决的问题静态检查ruff / ESLint / TS 类型检查死代码、未定义变量、语法错误、可疑模式格式检查black / Prettier / gofmt格式不一致导致的 diff 噪音自动化测试pytest / Jest含覆盖率行为是否正确、回归是否引入提交钩子pre-commit 框架以上所有检查强制进仓库前执行这个项目是 Python 写的所以我的静态检查首选 ruff。它速度极快而且能识别很多 Pyflakes 和 pycodestyle 的规则。配置也简单pyproject.toml里写[tool.ruff] line-length 100 select [E, F, W, I, N, UP] [tool.ruff.format] quote-style double格式化用 black。很多人质疑格式化工具是不是多余的我的回答是它存在的意义不是让代码更好看而是让团队或者未来的你在 diff 时只看得到真实改动不用在这个缩进怎么换了上面浪费时间。4.2 pre-commit 钩子把检查焊死在提交环节工具再好如果靠人记着去跑终会遗漏。我把所有检查挂到 pre-commit 上提交时自动执行不通过就直接拒绝提交。.pre-commit-config.yaml配置长这样repos: - repo: https://github.com/astral-sh/ruff-pre-commit rev: v0.6.9 hooks: - id: ruff args: [--fix] - id: ruff-format - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.6.0 hooks: - id: check-merge-conflict - id: trailing-whitespace - id: check-yaml - id: debug-statements用 pre-commit 之后还有一个隐藏好处它强制你在提交前多花 30 秒思考。这 30 秒足以拦截掉 80% 的我先提了再说冲动。钩子里面我特别推荐debug-statements它会把print()pdbbreakpoint()这类调试残留直接拦下来。我曾经提交过一条print(response.json())到线上日志那段时间的日志文件长得惨不忍睹。这种问题靠人审查很难每次都抓到钩子是最后的防线。4.3 容易被忽视的两个小工具git diff --check 和 git status除了插件式检查git 自带的命令也很有用很多人根本不知道。git diff --check会检测 diff 里的空白错误——行尾空格、文件末尾缺换行等。这类问题一般不影响运行但会让 review 界面多出红色标记显得很不专业。我每次提交前必跑一次顺手就把问题消掉了。git status和git diff --stat更是老生常谈但最有用的命令。提交前看一眼工作区状态、确认没有把临时文件、配置文件、敏感信息一块提交进去。检查敏感信息这一点极其重要有一次我把API_KEY写进了配置文件提交到了仓库第二天才发现改 key 的过程痛苦得刻骨铭心。小项目没秘密保护机制自己提交前多看一眼比什么都强。4.4 打包进工作流的完整检查命令我现在提交前固定跑下面这串命令git diff --check ruff check . ruff format --check . pytest --covsnippet_store --cov-fail-under80 git status git diff --stat执行顺序是有讲究的先查最便宜的diff 检查、ruff再跑功耗最高的pytest 全量。如果前面就挂了后面根本不用跑省时间。--cov-fail-under80是覆盖率阈值低于 80% 直接失败。这条不是为了让数字好看而是逼自己想清楚没测到的代码为什么不测很多隐藏 bug 就藏在未覆盖的分支里。5. 一次完整的提交前流水线登录、查漏、改造、验收、落地5.1 提交前先登录明确本次提交的目标我们项目最近的改动是给片段添加上次使用时间。我把它作为一次提交前流水线的实例把整个流程走一遍给你看。第一步不是写代码而是明确这次提交要完成的目标。这个目标相当于登录一个问题域——一句话说清楚本次改动做什么目标为片段增加上次使用时间在搜索命中时更新时间戳不影响现有搜索行为和统计逻辑。目标里写清楚不影响什么很重要它能防止你在开发过程中顺手改掉别的东西。确认目标后新建分支git checkout -b feat/update-last-used-at分支名有命名规则feat/、fix/、refactor/、chore/每个前缀对应这次提交属于哪一类提交记录可以按类型快速筛选。5.2 先跑一遍基线改之前必须知道当前状态动手之前我先跑一遍全量测试和 lint记录当前状态。这不是走形式是给自己留一个改动前基线pytest --covsnippet_store --cov-fail-under80 # 20 passed, 98% coverage ruff check . # All checks passed!基线的意义在于如果改完后测试挂了你能确定是自己的改动引入的而不是本来就有问题。你会发现这一步在团队项目里有 CI 兜底自己写项目时就只能靠这根救命绳。5.3 开发完成后按审查清单逐项过 diff功能写完自测通过进入审查环节。我打开git diff逐块看同时打开审查清单把每一项往代码上套。这次改动主要涉及两个地方snippet_store.py里新增了一个update_last_used函数以及main.py里调用它。审查结果记录如下正确性update_last_used在片段不存在时会触发KeyError但调用方已经提前用get_snippet()判断过存在性风险可控。健壮性时间使用datetime.now().isoformat()存储查询时用字符串比较格式统一可行。可读性函数命名清晰docstring 写清楚了时间格式和更新规则通过。性能每次搜索命中都要写一次时间戳频率高时磁盘 IO 会变大暂时可接受但记到 TODO 里后续做批量落盘。安全无新的输入注入点通过。这个步骤不允许我自己骗自己说差不多可以了因为审查的意义就是把判断过程显性化。5.4 检查链全跑一遍写清楚提交信息确认没问题后跑全套检查链git diff --check ruff check . ruff format --check . pytest --covsnippet_store --cov-fail-under80全部绿最后一步是提交。提交信息我遵循约定的格式feat: 片段增加上次使用时间字段 - search_snippets 命中片段时自动更新 last_used_at - 新增 update_last_used 函数负责时间戳写入 - 补齐相关单元测试覆盖片段不存在的边界 注高频命中时磁盘 IO 偏高后续可改为批量落盘再优化。提交信息的价值在于以后你翻git log时看到的不是一串update code这样的信息而是一段能独立理解的历史。typescopedescription的结构让我在三个星期后看自己提交时不需要打开 diff 也能知道这次提交做了什么、为什么做。5.5 最后一步干净环境验证消灭我机器上没问题提交前最后一道工序也是我自己吃过亏之后才养成的习惯在临时目录里克隆仓库跑一遍核心命令模拟新鲜环境。cd /tmp git clone ~/projects/snippet-manager fresh-check cd fresh-check python -m venv .venv .venv/bin/pip install -e . .venv/bin/snippet-manager search python --tag cli这一步专门抓那些依赖没打全路径写死配置文件缺失的问题。别小看它我不少项目都是靠这一步拦住原来仓库里根本没把requirements.txt更新这种低级问题的。干净环境验证通过之后才能算这个提交真正完成了。推送收工。6. 当项目长大以后我的三个顺手技巧和两个大坑6.1 技巧一把提交记录当成代码来审查代码审查不只看代码本身提交信息也值得审。我给自己定的标准是如果三周后的我拿着这个 commit message能否在不看 diff 的情况下理解这次改动的意图如果不能说明信息没写清楚。比如fix bug这种提交信息就是垃圾它没有描述 bug 是什么、怎么修、影响面多大。好的提交信息是一个微型文档是你能留给未来自己的最好注释。我自己越来越倾向于在 message 里写三段式一句话目标、具体改动列表、备注已知风险/后续计划。写多了之后甚至能直接从git log里梳理出项目演进史。6.2 技巧二先写失败用例再做修复修 bug 时我的习惯是先写一个能复现问题的测试用例——它在这个 bug 存在时是红失败的修复后才变绿。这个过程等于先给 bug 拍了照片再慢慢动手治。好处有两点你确切知道自己修的 bug 是什么而不是改了感觉应该没问题。这个测试会一直留在测试集里成为防止回归的守门员。下次有人改了逻辑忘掉这个行为测试会拦下他。拿前面search_snippets()那个隐藏 bug 来说我重构前先写了test_search_empty_query_returns_empty它跑红了然后才动手修逻辑。这一步让重构有了一个明确的目标函数——看完测试运行结果的变化你就知道改动是否真的生效了。6.3 技巧三提交前跑一次临时分支重放还有一个我强烈推荐的方法在提交前把改动内容分布到临时分支重新模拟功能原本的样子来验证。做法是把当前改动git stash。从上一个提交切一个新分支tmp/verify。把stash恢复应用到新分支。在新分支上跑全量测试和主流程。如果这一步绿了说明你的改动是完整自洽的如果挂了说明你依赖了未提交的其他改动。这个技巧能抓住那种提交 A 和提交 B 分开看起来都对合起来就崩的依赖问题。很多日常团队里常见的坑其实靠这一步就能提前发现。6.4 大坑一格式化自动刷全场diff 灾难现场这个坑发生在我把ruff format挂在 pre-commit 之后有一次提交时pre-commit 把整个项目的字符串引号从单引号改成双引号然后我的 diff 瞬间变成了几百行真正的改动淹没在格式化噪音里。更麻烦的是团队其他人一看 diff 根本不知道我改了啥。解决方法是格式化工具的配置在项目初始就该决定好并固定如果你在项目中期引入 ruff 这类工具第一次运行往往需要单独跑一个style 改造提交把格式化工作独立出来不跟功能改动挂钩。后续再改代码你的 diff 就清爽干净了。6.5 大坑二审查和重构同时开工身份错乱有一次我一边审查一边顺手重构改着改着把本来要保留的一个特性改没了。原因是我在看代码时发现这个分支看起来没用手一抖就删了后来才知道那是给移动端预留的兼容逻辑。回顾后我给自己定了一条规矩审查时只记录不改动重构时只动结构不加逻辑。审查时 发现问题 - 记进清单写清楚行号、问题、建议 如果确认要改 - 先加 TODO 标记等重构阶段统一处理 重构时 打开清单逐条处理 不新开问题不顺手改别处 改完一个勾掉一个这条纪律看起来很死板但它能把审查和重构这两个动作的边界划清楚。两个动作一旦混在一起责任就说不清了bug 也会悄悄从缺口里溜进来。说实话第一次完整走完这套流程我的感觉是多花了二十分钟。但隔了两周再回去改那段代码我突然意识到省下的远不止二十分钟——因为代码结构清晰、行为有测试兜底、提交记录可追溯我几乎不用再花时间去理解我当初为什么这么写。小项目的优雅不是一次写对而是在提交前把该做的三件事做扎实。现在的我宁可提交慢一点也再也不想体验那次倒车事故了。