代码评审实战：从合并冲突到架构反馈的工程协作

代码评审实战：从合并冲突到架构反馈的工程协作

一、代码评审的真实场景：不是找 Bug，是找设计问题

实习期间最被低估的技能不是写代码，而是代码评审。很多新人以为 Code Review 就是找语法错误和命名不规范，但实际上评审的核心价值是发现设计问题：接口定义是否合理、状态管理是否清晰、错误处理是否完备。一个设计良好的接口可以避免后续的重构成本，而命名问题只是表面功夫。

合并冲突是代码评审中最常见的协作问题。两个人同时修改同一个文件，Git 无法自动合并时就需要手动解决。但解决冲突不只是"保留哪段代码"，更需要理解两段修改的意图，判断是否可以共存。

二、代码评审的检查框架

graph TB CR[Code Review] --> CORRECT[正确性 逻辑是否正确] CR --> DESIGN[设计性 接口是否合理] CR --> ROBUST[健壮性 异常处理是否完备] CR --> MAINTAIN[可维护性 代码是否易读易改] CORRECT --> C1[边界条件] CORRECT --> C2[并发安全] CORRECT --> C3[类型安全] DESIGN --> D1[接口粒度] DESIGN --> D2[职责划分] DESIGN --> D3[扩展性] ROBUST --> R1[空值处理] ROBUST --> R2[超时重试] ROBUST --> R3[降级策略] MAINTAIN --> M1[命名清晰] MAINTAIN --> M2[注释适度] MAINTAIN --> M3[复杂度控制]

三、代码评审的实战案例

# ========== 案例 1：接口设计问题 ========== # ❌ 问题代码：接口职责不清晰 def process_data(data, save_to_db=True, send_email=True, format="json"): """处理数据：同时负责解析、存储和通知""" parsed = parse(data, format) if save_to_db: db.save(parsed) # 数据库操作和业务逻辑耦合 if send_email: email.send(parsed) # 通知逻辑和核心逻辑耦合 return parsed # ✅ 评审建议：拆分职责，单一职责原则 def parse_data(data, format="json"): """只负责解析数据""" return parse(data, format) def save_data(parsed_data): """只负责持久化""" db.save(parsed_data) def notify_data(parsed_data): """只负责通知""" email.send(parsed_data) # 调用方按需组合 parsed = parse_data(raw_data) save_data(parsed) notify_data(parsed) # ========== 案例 2：并发安全问题 ========== # ❌ 问题代码：非线程安全的计数器 class Counter: def __init__(self): self.count = 0 def increment(self): self.count += 1 # 非原子操作：读→加→写 # ✅ 评审建议：使用锁或原子操作 import threading class ThreadSafeCounter: def __init__(self): self._count = 0 self._lock = threading.Lock() def increment(self): with self._lock: self._count += 1 @property def count(self): with self._lock: return self._count # ========== 案例 3：合并冲突解决 ========== # 开发者 A 的修改：增加日志 def get_user(user_id): logger.info(f"Fetching user {user_id}") # A 新增 user = db.query("SELECT * FROM users WHERE id = ?", user_id) return user # 开发者 B 的修改：增加缓存 def get_user(user_id): cached = redis.get(f"user:{user_id}") # B 新增 if cached: return cached user = db.query("SELECT * FROM users WHERE id = ?", user_id) return user # ✅ 合并后：两者意图可以共存 def get_user(user_id): logger.info(f"Fetching user {user_id}") # A 的日志 cached = redis.get(f"user:{user_id}") # B 的缓存 if cached: return cached user = db.query("SELECT * FROM users WHERE id = ?", user_id) return user # ========== 案例 4：Review Comment 模板 ========== REVIEW_TEMPLATES = { "design": "建议：{suggestion}。当前设计的考虑是{reason}，但可能导致{risk}。", "robustness": "风险：{scenario} 场景下可能{failure}。建议增加{fix}。", "naming": "命名建议：{current} → {suggested}，原因：{why}。", "complexity": "复杂度：当前 O({current})，可通过{technique}优化到 O({target})。", }

四、代码评审的 Trade-offs 分析

评审深度与效率：逐行 Review 深度高但效率低，一个 500 行的 PR 可能需要 1 小时。建议分层评审：先看接口设计和状态管理（10 分钟），再看核心逻辑（20 分钟），最后看命名和风格（快速扫过）。

评审语气与团队氛围：过于尖锐的评审意见会打击新人积极性。建议用"建议"而非"必须"，用"考虑"而非"应该"。评审的目标是提升代码质量，不是证明自己比对方强。

自动化与人工的边界：ESLint 和 TypeScript 可以自动检查风格和类型问题，人工 Review 应该聚焦在自动化工具无法覆盖的设计和逻辑问题。不要在 Review 中指出 ESLint 能自动修复的问题。

PR 粒度的选择：大 PR（500+ 行）Review 质量低，小 PR（50 行）Review 质量高但数量多。建议单个 PR 不超过 300 行，超过时拆分为多个逻辑独立的 PR。

五、总结

代码评审的核心价值不是找 Bug 而是发现设计问题。四个检查维度（正确性、设计性、健壮性、可维护性）覆盖了从逻辑到架构的全层次。合并冲突解决需要理解双方意图而非简单保留。评审语气影响团队氛围，建议用建设性语言。自动化工具处理风格问题，人工 Review 聚焦设计和逻辑。建议控制 PR 粒度在 300 行以内，提升 Review 质量。