rt-thread icon indicating copy to clipboard operation
rt-thread copied to clipboard

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

Open unicornx opened this issue 1 year ago • 6 comments

Describe problem solved by the proposed feature

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

Describe your preferred solution

部分解决方案见问题描述

Describe possible alternatives

No response

unicornx avatar Jul 04 '24 00:07 unicornx

很好的建议,RTT是逐步发展起来的,希望可以建立好的管理制度。也希望loop更多人一起来把关,例如分级、分子系统的maintainer制度

BernardXiong avatar Jul 04 '24 01:07 BernardXiong

我觉得 问题5 中的 maintainer 制度是否可以优先建立起来,只有找对合适的,负责任的人,并逐级推广之,其他问题都能慢慢解决。

unicornx avatar Jul 07 '24 00:07 unicornx

good idea,但现实情况现在开源社区贡献的人,能力参差不齐,有能力把关且愿意参与进来的更少,需要跟多无私奉献的。

ComerLater avatar Jul 07 '24 08:07 ComerLater

good idea,但现实情况现在开源社区贡献的人,能力参差不齐,有能力把关且愿意参与进来的更少,需要跟多无私奉献的。

是的,国内开源社区还需要培养,"不忘初心,砥砺前行" 😄

unicornx avatar Jul 08 '24 00:07 unicornx

@unicornx 可以帮忙整理出一定的commit,PR的规范吗?如能形成issue,可以钉在置顶上。谢谢

BernardXiong avatar Jul 21 '24 01:07 BernardXiong

@unicornx 可以帮忙整理出一定的commit,PR的规范吗?如能形成issue,可以钉在置顶上。谢谢

好的,我找时间来整理一下

unicornx avatar Jul 22 '24 00:07 unicornx

现在有maintainer制度了,希望每个maintainer 能把把关,close first。

unicornx avatar Sep 02 '25 01:09 unicornx