docs/maintainers/PR_REVIEW_GUIDE.zh-CN.md
本指南适用于审核 pull request 的 NOFX 维护者。
检查 PR 与路线图的一致性
验证 PR 完整性
应用适当的标签
分配审核者
✅ **要问的问题：**
- 是否解决了所述问题？
- 边界情况是否处理？
- 是否会破坏现有功能？
- 方法是否适合我们的架构？
- 是否有更好的替代方案？
测试：
Go 后端代码：
// ❌ 差 - 拒绝
func GetData(a, b string) interface{} {
d := doSomething(a, b)
return d
}
// ✅ 好 - 批准
func GetAccountBalance(apiKey, secretKey string) (*Balance, error) {
if apiKey == "" || secretKey == "" {
return nil, fmt.Errorf("API credentials required")
}
balance, err := client.FetchBalance(apiKey, secretKey)
if err != nil {
return nil, fmt.Errorf("failed to fetch balance: %w", err)
}
return balance, nil
}
检查项：
TypeScript/React 前端代码：
// ❌ 差 - 拒绝
const getData = (data: any) => {
return data.map(d => <div>{d.name}</div>)
}
// ✅ 好 - 批准
interface Trader {
id: string;
name: string;
status: 'running' | 'stopped';
}
const TraderList: React.FC<{ traders: Trader[] }> = ({ traders }) => {
return (
<div className="trader-list">
{traders.map(trader => (
<TraderCard key={trader.id} trader={trader} />
))}
</div>
);
};
检查项：
any）关键检查：
// 🚨 拒绝 - 安全问题
func Login(username, password string) {
query := "SELECT * FROM users WHERE username='" + username + "'" // SQL 注入！
db.Query(query)
}
// ✅ 批准 - 安全
func Login(username, password string) error {
query := "SELECT * FROM users WHERE username = ?"
row := db.QueryRow(query, username) // 参数化查询
// ... 使用 bcrypt 进行正确的密码验证
}
使用这些标准来分配优先级：
Critical（严重）：
High（高）：
Medium（中）：
Low（低）：
needs review → in review → needs changes → needs review → approved → merged
↓
on hold
状态标签：
status: needs review - 准备初次审核status: in progress - 正在积极审核status: needs changes - 审核者请求更改status: on hold - 等待讨论/决定status: blocked - 被另一个 PR/issue 阻塞❌ 差的评论：
这是错的。
改这个。
你为什么这样做？
✅ 好的评论：
这种方法可能会导致并发请求的问题。
考虑在这里使用互斥锁或原子操作。
建议：将此逻辑提取到单独的函数中以提高可测试性：
```go
func validateTraderConfig(config *TraderConfig) error {
// 验证逻辑
}
问题：你是否考虑过使用现有的 ExchangeClient 接口
而不是创建新接口？这将与代码库的其余部分保持一致。
### 评论类型
**🔴 阻塞性（必须解决）：**
```markdown
**阻塞性：** 这引入了 SQL 注入漏洞。
请改用参数化查询。
🟡 非阻塞性（建议）：
**建议：** 考虑在这里使用 `strings.Builder` 以提高
连接多个字符串时的性能。
🟢 赞扬（鼓励好的做法）：
**很好！** 很好地使用 context 进行超时处理。这正是
我们想看到的。
❌ 指令（可能感觉强硬）：
改用工厂模式。
为这个函数添加测试。
✅ 问题（更协作）：
工厂模式在这里会更合适吗？它可能会使测试更容易。
你能为错误路径添加一个测试用例吗？我想确保我们
优雅地处理失败。
| PR 类型 | 初次审核 | 后续审核 | 合并决定 |
|---|---|---|---|
| 严重 Bug | 4 小时 | 2 小时 | 当天 |
| 悬赏 PR | 24 小时 | 12 小时 | 2-3 天 |
| 功能 | 2-3 天 | 1-2 天 | 3-5 天 |
| 文档 | 2-3 天 | 1-2 天 | 3-5 天 |
| 大型 PR | 3-5 天 | 2-3 天 | 5-7 天 |
PR 应在以下情况下批准：
功能性
质量
安全性
文档
流程
在以下情况下拒绝 PR：
立即拒绝：
请求更改：
关闭并说明：
需要额外注意：
满足以下条件时合并：
Squash Merge（大多数 PR 的默认策略）：
Merge Commit（复杂 PR）：
Rebase and Merge（很少使用）：
格式：
<type>(<scope>): <PR 标题> (#123)
变更的简要描述。
- 关键变更 1
- 关键变更 2
Co-authored-by: 贡献者姓名 <[email protected]>
每月监控这些指标：
如果对 PR 不确定：
记住： 审核应该是尊重的、建设性的和教育性的。 我们在构建社区，而不仅仅是代码。🚀