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

feat: CI script assigns PR reviews based on the list of maintainers #9913

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kurisaW
Copy link
Contributor

@kurisaW kurisaW commented Jan 14, 2025

Fixed #9903

Ps:该CI自动化脚本可自动识别PR修改的文件列表,并根据仓库根目录下的MAINTAINERS文件,映射审核团所维护的path,从而分配审查

下面是该脚本的具体功能说明:

  • 审核方式由github bot自动评论@提及,并且保证仅保留一份最新的机器人评论;
  • 支持动态检测PR内所有被修改、删除、增加的文件;支持被修改文件的递归目录检测;并根据每次最新PR所变更的所有文件内容的追踪,生成实时的审核内容及审查人任命;
  • 支持查看最新PR的审查情况,可根据CI机器人评论的Current Review Status部分查看;
  • 支持自动捕获被任命的评论信息(审核者需要评论 LGTM/lgtm 来告知审查完毕),CI会同步bot机器人的评论:Current Review Status更新审查状况;
  • 支持手动点击🔄 刷新状态来重新运行CI,以此来获取最新的审查情况;
  • 支持一键展开被修改的文件详情,方便定位审查内容。

此外,为避免因PR多次修改而重复@审核者,该脚本还引入缓存机制,仅在第一次@时评论提及,后续仅提示GitHub id

下面是一个演示效果,也可以通过下方链接查看实际效果:

📌 Code Review Assignment

🏷️ Tag: stm32

Path: bsp/stm32
Reviewers: @kurisaW

Changed Files (Click to expand)
  • bsp/stm32/docs/How to create a RT-Studio project.md

🏷️ Tag: kernel

Path: src
Reviewers: @kurisaW @Nedki-L

Changed Files (Click to expand)
  • src/Kconfig

🏷️ Tag: workflow

Path: .github/workflows
Reviewers: @kurisaW @Nedki-L @KurisaW-Collaborative

Changed Files (Click to expand)
  • .github/workflows/auto-assign-reviewers.yml

📊 Current Review Status (Last Updated: 2025-01-23 06:31 UTC)

  • kurisaW Reviewed On 2025-01-23 03:43 UTC
  • Nedki-L Reviewed On 2025-01-23 04:04 UTC
  • KurisaW-Collaborative Pending Review

📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

@kurisaW kurisaW requested a review from supperthomas as a code owner January 14, 2025 05:48
@github-actions github-actions bot added the action github action yml imporve label Jan 14, 2025
@kurisaW kurisaW marked this pull request as draft January 14, 2025 05:51
@kurisaW kurisaW force-pushed the auto-reviewer branch 3 times, most recently from 3e75f0a to 8d799f2 Compare January 14, 2025 06:09
@kurisaW kurisaW marked this pull request as ready for review January 14, 2025 06:20
@Rbb666 Rbb666 added action github action yml imporve and removed action github action yml imporve labels Jan 14, 2025
@Rbb666 Rbb666 requested a review from mysterywolf January 14, 2025 06:30
@Rbb666 Rbb666 added the BSP: Renesas BSP related with Renesas label Jan 14, 2025
@Rbb666 Rbb666 marked this pull request as draft January 14, 2025 07:05
@supperthomas
Copy link
Member

你这个要加RTTHREAD_GITHUB_TOKEN的吧

@kurisaW
Copy link
Contributor Author

kurisaW commented Jan 14, 2025

你这个要加RTTHREAD_GITHUB_TOKEN的吧

token加了的:

image

但是还不清楚为什么这个仓库解析出了问题:

image

然后在我自己的仓库测试中是可以的,等我找时间再看看:

image

@supperthomas
Copy link
Member

目前这个仓库里面的RTTHREAD_GITHUB_TOKEN 加了吗?还没有加吧?

@kurisaW
Copy link
Contributor Author

kurisaW commented Jan 15, 2025

目前这个仓库里面的RTTHREAD_GITHUB_TOKEN 加了吗?还没有加吧?

应该是有的吧,我是看其他ci文件中有这个token在,然后用的这个,不知道我上面的问题会不会是因为这个token无效导致的

image

@supperthomas
Copy link
Member

目前这个仓库里面的RTTHREAD_GITHUB_TOKEN 加了吗?还没有加吧?

有的,我是看ci文件中有这个token在,然后用的这个,不然没法通过权限的

没有的,那个是我加的。token你找熊大加一下。

@kurisaW
Copy link
Contributor Author

kurisaW commented Jan 15, 2025

目前这个仓库里面的RTTHREAD_GITHUB_TOKEN 加了吗?还没有加吧?

有的,我是看ci文件中有这个token在,然后用的这个,不然没法通过权限的

没有的,那个是我加的。token你找熊大加一下。

噢噢好的,那我另设一个

@supperthomas
Copy link
Member

目前这个仓库里面的RTTHREAD_GITHUB_TOKEN 加了吗?还没有加吧?

有的,我是看ci文件中有这个token在,然后用的这个,不然没法通过权限的

没有的,那个是我加的。token你找熊大加一下。

噢噢好的,那我另设一个

不是我的意思是你还是用RTTHREAD_GITHUB_TOKEN 这个就可以了。这个settings里面的值没有赋值。

@kurisaW
Copy link
Contributor Author

kurisaW commented Jan 15, 2025

@BernardXiong 熊大,麻烦帮忙添加个名为 GITHUB_TOKEN_COMMENT 的token可以嘛,只需要拥有对仓库的评论权限就可以了

@kurisaW kurisaW marked this pull request as ready for review January 16, 2025 01:14
@kurisaW kurisaW closed this Jan 16, 2025
@kurisaW kurisaW reopened this Jan 16, 2025
@unicornx
Copy link
Contributor

unicornx commented Jan 20, 2025

我想了解一下这个工作是否可以和 #9903 结合起来?

具体来说,就是我想问一下,我们是否可以维护一个 MAINTAINER 文件(MAINTAINER 文件中记录了 reviewer 的相关信息),然后利用 MAINTAINER 文件 中的信息来指定 reviewer?

为啥要用 label 信息?感觉 label 信息以后扩展方便吗?

@kurisaW kurisaW marked this pull request as draft January 20, 2025 04:41
@kurisaW kurisaW changed the title ci: appoint PR reviewers via action bot reviews feat: CI script assigns PR reviews based on the list of maintainers Jan 23, 2025
@kurisaW kurisaW force-pushed the auto-reviewer branch 3 times, most recently from 70252d0 to 97ebf5a Compare January 23, 2025 09:22
MAINTAINERS Show resolved Hide resolved
MAINTAINERS Show resolved Hide resolved
@unicornx
Copy link
Contributor

unicornx commented Feb 6, 2025

  • owner 字段可以用括号或者尖括号分隔,只是不加空格人看起来会觉得拥挤。当然我觉得不加也可以,毕竟有 ()和 <> 分隔也能解析
  • 希望在 MAINTAINERS 上加上注释,注释相当于MAINTAINERS 的语法规则,我们好 review。目的是为了方便以后大家修改这个 MAINTAINERS 文件。因为以后我理解如果一个人想当某个模块的 maintainer 是会自己提pr来改这个文件的,所以我们需要给他一个修改说明指导,今天我们在 issue 上记录的这些讨论过程最好形成文字留下来,否则以后谁会再解释?
  • 多个 owner 的问题我看了你的例子,我没有啥问题了,请在注释中说明我们是支持多个 owner 的
  • 有关 tag 的问题,我记得满老师提这个是为了说明每个部分的描述,建议保留。需要注意的是 tag 里允许出现空格吗?原来满老师的例子感觉用json 的 tag 是双引号括起来,而且是允许有空格的,不知道你这里改了以后有什么限制,请确认。
  • 我们这里 path 和 tag 怎么保证唯一?如果写重了怎么办?

@kurisaW
Copy link
Contributor Author

kurisaW commented Feb 6, 2025

  • owner 字段可以用括号或者尖括号分隔,只是不加空格人看起来会觉得拥挤。当然我觉得不加也可以,毕竟有 ()和 <> 分隔也能解析
  • 希望在 MAINTAINERS 上加上注释,注释相当于MAINTAINERS 的语法规则,我们好 review。目的是为了方便以后大家修改这个 MAINTAINERS 文件。因为以后我理解如果一个人想当某个模块的 maintainer 是会自己提pr来改这个文件的,所以我们需要给他一个修改说明指导,今天我们在 issue 上记录的这些讨论过程最好形成文字留下来,否则以后谁会再解释?
  • 多个 owner 的问题我看了你的例子,我没有啥问题了,请在注释中说明我们是支持多个 owner 的
  • 有关 tag 的问题,我记得满老师提这个是为了说明每个部分的描述,建议保留。需要注意的是 tag 里允许出现空格吗?原来满老师的例子感觉用json 的 tag 是双引号括起来,而且是允许有空格的,不知道你这里改了以后有什么限制,请确认。
  • 我们这里 path 和 tag 怎么保证唯一?如果写重了怎么办?

回复提出的问题:
1.owner支持通过加入空格的形式,如 user1 (user1 id) [email protected],但我个人感觉 user1(user1 id)[email protected]的效果也还可以,加上空格反而看起来更散了
2.MAINTAINER已增加注释部分,同时附带新增maintainer说明
3.注释中已说明
4.tag是允许支持各种特殊字符的,并不会分割字符内容
5.一般来说tag的范围应该大于path,所以也会存在多个path可以同用一个tag的情况,后续再支持一个tag对应多个path的功能

@kurisaW kurisaW marked this pull request as ready for review February 6, 2025 03:57
@kurisaW kurisaW requested a review from unicornx February 6, 2025 05:18
MAINTAINERS Outdated Show resolved Hide resolved
MAINTAINERS Show resolved Hide resolved
MAINTAINERS Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action github action yml imporve BSP: Renesas BSP related with Renesas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] 为 RTT 建立 maintainer 机制
4 participants