Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
unicornx opened this issue Jul 4, 2024 · 6 comments
Assignees

Comments

@unicornx
Copy link
Contributor

unicornx commented Jul 4, 2024

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

@BernardXiong
Copy link
Member

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

@unicornx
Copy link
Contributor Author

unicornx commented Jul 7, 2024

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

@Later-Comer
Copy link
Contributor

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

@unicornx
Copy link
Contributor Author

unicornx commented Jul 8, 2024

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

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

@BernardXiong
Copy link
Member

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

@unicornx
Copy link
Contributor Author

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

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

@unicornx unicornx self-assigned this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants