## PR 审查 通过 Pull Request 的系统化审查确保代码质量和架构健全性。 ### 使用方法 ```bash # PR 的全面审查 gh pr view 123 --comments "系统化审查这个 PR,从代码质量、安全性、架构角度提供反馈" # 安全性专注审查 gh pr diff 123 "专注于安全风险和漏洞进行审查" # 架构视角的审查 gh pr checkout 123 && find . -name "*.js" | head -10 "从层级分离、依赖关系、SOLID 原则的角度评估架构" ``` ### 基本示例 ```bash # 代码质量的数值评估 find . -name "*.js" -exec wc -l {} + | sort -rn | head -5 "评估代码的复杂度、函数大小、重复度并指出改进点" # 安全漏洞检查 grep -r "password\|secret\|token" . --include="*.js" | head -10 "检查敏感信息泄露、硬编码、认证绕过的风险" # 架构违规检测 grep -r "import.*from.*\\.\\./\\.\\." . --include="*.js" "评估层级违规、循环依赖、耦合度问题" ``` ### 评论分类体系 ```text 🔴 critical.must: 致命问题 ├─ 安全漏洞 ├─ 数据一致性问题 └─ 系统故障风险 🟡 high.imo: 高优先级改进 ├─ 功能故障风险 ├─ 性能问题 └─ 可维护性大幅降低 🟢 medium.imo: 中优先级改进 ├─ 可读性提升 ├─ 代码结构改进 └─ 测试质量提升 🟢 low.nits: 轻微指摘 ├─ 风格统一 ├─ 拼写错误修正 └─ 注释添加 🔵 info.q: 问题·信息提供 ├─ 实现意图确认 ├─ 设计决策背景 └─ 最佳实践分享 ``` ### 审查观点 #### 1. 代码正确性 - **逻辑错误**: 边界值、Null 检查、异常处理 - **数据一致性**: 类型安全、验证 - **错误处理**: 全面性、适当处理 #### 2. 安全性 - **认证·授权**: 适当检查、权限管理 - **输入验证**: SQL 注入、XSS 对策 - **敏感信息**: 禁止日志输出、加密 #### 3. 性能 - **算法**: 时间复杂度、内存效率 - **数据库**: N+1 查询、索引优化 - **资源**: 内存泄漏、缓存利用 #### 4. 架构 - **层级分离**: 依赖方向、适当分离 - **耦合度**: 松耦合、接口使用 - **SOLID 原则**: 单一职责、开闭原则、依赖倒置 ### 审查流程 1. **事前确认**: PR 信息、变更差异、相关 Issue 2. **系统化检查**: 安全性 → 正确性 → 性能 → 架构 3. **建设性反馈**: 具体改进建议和代码示例 4. **后续跟进**: 修正确认、CI 状态、最终批准 ### 评论模板 #### 安全问题模板 **格式:** - 严重程度:`critical.must.` - 问题:明确描述问题 - 代码示例:提供修复方案 - 理由:解释为什么需要修复 **示例:** ```text critical.must. 密码以明文保存 修复方案: const bcrypt = require('bcrypt'); const hashedPassword = await bcrypt.hash(password, 12); 为防止安全风险,必须进行哈希处理。 ``` #### 性能改进模板 **格式:** - 严重程度:`high.imo.` - 问题:解释性能影响 - 代码示例:提供改进方案 - 效果:描述预期改进 **示例:** ```text high.imo. 会发生 N+1 查询问题 改进方案:Eager Loading const users = await User.findAll({ include: [Post] }); 可以大幅减少查询数量。 ``` #### 架构违规模板 **格式:** - 严重程度:`high.must.` - 问题:指出架构原则违规 - 解决方案:提供架构改进方向 - 原则:引用相关设计原则 **示例:** ```text high.must. 发生了层级违规 领域层直接依赖基础设施层。 请通过依赖倒置原则引入接口。 ``` ### 注意事项 - **建设性语气**: 协作而非攻击性的沟通 - **具体建议**: 不仅指出问题,还要提供解决方案 - **优先级排序**: 按 Critical → High → Medium → Low 顺序处理 - **持续改进**: 将审查结果知识库化