谷动谷力

 找回密码
 立即注册
查看: 2116|回复: 0
打印 上一主题 下一主题
收起左侧

如何快速向RT-Thread提一个PR:以CI为例

[复制链接]
跳转到指定楼层
楼主
发表于 2023-6-9 22:46:31 | 只看该作者 |只看大图 回帖奖励 |倒序浏览 |阅读模式
本帖最后由 sunsili 于 2024-1-6 18:07 编辑

如何快速向RT-Thread提一个PR:以CI为例


PR方法(Git操作)


01Fork

首先将官方仓库 fork 到我们自己的账号下,点击一下 fork 按钮,按照提示操作即可。
回到自己的仓库中,将仓库clone到本地。
如果是较早之前fork的仓库,可以先和上游的仓库同步一下:
1. 先在仓库页面Update branch
2. 在本地仓库git fetch & git merge



02Commit & Push

一般来说,一个issue或者PR对应一个新的分支,所以需要创建一个分支
  1. git checkout -b <branchname>
复制代码

更改相应的代码,然后提交代码
  1. git add .
  2. git commit -m "message"
  3. git push origin test-branch
复制代码


03Pull Request

在仓库首页按操作提起一个PR

04Rollback

如果提交有误或者需要修改,可以进行回滚,然后重新push
  1. git reset HEAD^(回退到上一个版本)
复制代码

05CI

查看CI结果,根据结果对代码进行修改


功能的修改和增加:以CI(issue 7458)为例


01理解issue

[CI][cppcheck] 需要根据ignore_format 过滤掉不需要扫描的文件夹
· Issue #7458
· RT-Thread/rt-thread (github.com)https://github.com/RT-Thread/rt-thread/issues/7458

对问题本身的理解十分重要,issue里可能会有其他人对这个需求的或者问题详细讨论和思路的提供,所以仔细查看issue非常重要。
根据这个issue的描述,主要是添加忽略一些文件和目录进行静态检查的功能。
具体需求是:
1. 添加对.ignore_format.yml配置文件检查
2. 如果新更新的文件在dir_path属性的目录下或者在file_path属性中
3. 对这个文件就不启用静态检查
并且这个issue中也提供了一种思路:
1. 遍历一下ignore_format.yml文件找到所有需要忽略的文件夹,搞个大数组,然后过滤
2. 参考一下
https://github.com/RT-Thread/rt-thread/blob/master/tools/file_check.py

02代码定位

代码定位是对功能的修改和增加的第一步,对于Bug fix来说,代码定位可能比较困难,但是对于功能的修改和增加来说,是比较简单的。
首先可以看出来这个issue的工作主要是CI这块,那么我们需要先了解github的CI工具:Github Action。最好的文档肯定就是GitHub Actions文档 - GitHub 文档。我们不需要全部了解之后再动手,只要了解一些基本概念之后就可以先动手。
根据文档的描述,我们可以了解到github workflow使用yml来描述,并且放在了.github/workflow目录下,所以第一步可以定位到.github/workflow下的static_code_analysis.yml。
在这个目录我们可以看到static_code_analysis.yml是直接在yaml中用shell编写工作流程,而还有其它文件如file_check.yml使用调用其它Python脚本来完成工作流程。所以基本可以确定我们需要修改或者增加的地方就在.github/workflow和tools/ci下。

03代码阅读

在这里,我们参考file_check.yml的实现。所以需要先理解file_check.yml的整体实现。
file_check.yml主要先使用shell按照了Pyhon脚本中必须使用的库然后直接调用Python脚本,我们直接跳入这个被调用的脚本查看。
代码阅读个人比较喜欢也觉得比较快速的方式是:先了解代码的某一部分(需要控制精度)的大致结构和功能,不关注其具体实现,之后再深入理解具体实现。在这个过程中有需要去猜测别人是如何写的代码,并且在一步步阅读代码的过程当中验证和纠正自己的想法。
我们来看file_check.py。
我们可以先忽略使用的click命令行库,或者也可以从命名和使用方式猜测出它们的功能。
因为这个文件比较简单,所以我们可以猜测函数的入口就是check()。主函数里的逻辑是十分简单的,可以看到通过checkout.get_new_file()获得了一个文件列表,然后传递给了FormatCheck和LicenseCheck,它们又分别调用了自身的check函数,最后根据它们返回值判断是否检查出错误。
所以我们可以猜测checkout.get_new_file()获得的文件列表是需要检查的文件列表,而FormatCheck和LicenseCheck执行各自的检查逻辑,我们可以不用关注。
而get_new_file的就需要深入代码看具体实现,具体的逻辑也比较简单。
1. 通过git命令获得新增和修改的文件列表
2. 然后遍历这个文件列表
3. 遍历这个文件列表中的文件路径的每一层目录,看是否存在.ignore_format.yml文件
4. 然后根据.ignore_format.yml的属性来判断当前文件是否需要被检查
所以我们实现的重点就是对需要检查的代码进行静态代码检查。

04功能增加

首先,因为获得需要检查的文件列表这个功能是可能会被多次利用,可以先提取出来作为一个独立功能,并且可以做一些优化(在获得新增和修改的文件列表时的写法可以优化)。
其次,最重要的就是利用cppcheck完成静态代码检查的功能:
1. 从文件列表中再一次过滤出C/C++相关文件
2. 然后使用cppcheck逐个检查文件列表,并且捕获标准错误流
  1. class CPPCheck:
  2.     def __init__(self, file_list):
  3.         self.file_list = file_list
  4.     def check(self):
  5.         file_list_filtered = [file for file in self.file_list if file.endswith(('.c', '.cpp', '.cc', '.cxx'))]
  6.         logging.info("Start to static code analysis.")
  7.         check_result = True
  8.         for file in file_list_filtered:
  9.             result = subprocess.run(['cppcheck', '--enable=warning', 'performance', 'portability', '--inline-suppr', '--error-exitcode=1', '--force', file], stdout = subprocess.PIPE, stderr = subprocess.PIPE)
  10.             logging.info(result.stdout.decode())
  11.             logging.info(result.stderr.decode())
  12.             if result.stderr:
  13.                 check_result = False
  14.         return check_result
  15. @click.group()
  16. @click.pass_context
  17. def cli(ctx):
  18.     pass
  19. @cli.command()
  20. def check():
  21.     """
  22.     static code analysis(cppcheck).
  23.     """
  24.     format_ignore.init_logger()
  25.     # get modified files list
  26.     checkout = format_ignore.CheckOut()
  27.     file_list = checkout.get_new_file()
  28.     if file_list is None:
  29.         logging.error("checkout files fail")
  30.         sys.exit(1)
  31.     # use cppcheck
  32.     cpp_check = CPPCheck(file_list)
  33.     cpp_check_result = cpp_check.check()
  34.     if not cpp_check_result:
  35.         logging.error("static code analysis(cppcheck) fail.")
  36.         sys.exit(1)
  37.     logging.info("check success.")
  38.     sys.exit(0)
  39. if __name__ == '__main__':
  40.     cli()
复制代码


05功能测试

完成代码的修改之后最重要的就是通过测试,最基本也是最简单的测试就是功能测试。所以我们可以给这次的修改安排三个测试:
修改cppcheck会出现错误的文件:
case 1:不将文件加入.ignore_format.yml,CI报错
case 2:将文件加入.ignore_format.yml的file_path,CI不报错
case 3:将文件加入.ignore_format.yml的dir_path,CI不报错


PR心得

这次PR的提交有以下两个小心得。

01仔细沟通

第一点,也是最重要的一点就是和主动社区的前辈进行交流,对issue的问题和需求进行讨论。在这个PR被merge之前我就完成了其余两版,但是因为缺乏沟通,不是很适合当前的RT-Thread。

02Github Action 本地测试

在修改CI部分时,每次都需要推送到远端才能执行相关的action,这样比较麻烦。可以使用nektos/act: Run your GitHub Actions locally

回复

使用道具 举报

您需要登录后才可以回帖 登录 | 立即注册

本版积分规则

QQ|Archiver|手机版|深圳市光明谷科技有限公司|光明谷商城|Sunshine Silicon Corpporation ( 粤ICP备14060730号|Sitemap

GMT+8, 2024-4-27 19:33 , Processed in 0.083357 second(s), 41 queries .

Powered by Discuz! X3.2 Licensed

© 2001-2013 Comsenz Inc.

快速回复 返回顶部 返回列表