Skip to content

[Feature] 对 PR 代码评审中存在问题的一些总结和改进建议,供参考 #9136

@unicornx

Description

@unicornx

Describe problem solved by the proposed feature

  • [问题1] 建议一个 PR 不要包含太多主题的修改,如果多个修改互相依赖需要作为一个 PR 整体合入的,建议将一个 PR 分解为多个 commit,按照依赖关系先后 commit。这个问题需要 author 和 reviewer 合作把关。参考 [bsp/milkv 仅用于讨论-后续拆分提交]为后续使用ioremap做准备 #9120 (comment) 中的 “第三个问题”
  • [问题 2],对于不需要像 [问题 1] 那样分 commit 处理的,提 PR 时 author 应该自己保证通过 rebase 压缩为一个 commit,PR 中不要保留开发中的中间历史记录,参考 [bsp/milkv 仅用于讨论-后续拆分提交]为后续使用ioremap做准备 #9120 (comment) 中的 “第四个问题”。
  • [问题 3],对于[问题 1] 形式的 PR,或者有时候是累积的多个修改作为一个 PR 提交的,maintainer 在 merge 时要保留 commit 的信息,不要简单地压缩。参考 Accumulated patchsets for bsp/cvitek #8968 (comment)
  • [问题 4], 我觉得目前 RTT 中提交的 PR 中的 commit 信息都写得过于简单了,这对项目的长期发展是不利的,对后来者阅读代码和理解修改历史很不友好。虽然现在 PR 中对描述问题有了一定的约束,其实我觉得更好的约束还是应该体现在 commit 信息中,因为 git 仓库和 PR 系统其实是分离的,如果 RTT 以后转移了托管(移到其他 譬如 gitee)或者单纯本地 git log,PR 中的信息就都没有了。详细的问题描述建议参考 [bsp/milkv 仅用于讨论-后续拆分提交]为后续使用ioremap做准备 #9120 (comment) 的 “第二个问题” 以及 [bsp/cvitek]将 eth 驱动中地址类型修改为指针以适应ioremap #9132 (comment)。这需要 maintainer 和 reviewer 严格把关。
  • [问题 5]:请问现在 RTT 是否有类似 Linux 的分级以及分子系统的 maintainer 管理制度,以及如果对某个分子系统有问题,我该去哪里查看可以寻求得到帮助?建议在 RTT 中建立类似 Linux 的 MAINTAINER 文件对以上信息进行维护。建立好后也可以对 PR 的 review 进行有效的分配和处理。

Describe your preferred solution

部分解决方案见问题描述

Describe possible alternatives

No response

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions