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

[CI] Added feature to filter out files to file_check.py that do not require cppcheck #7499

Merged
merged 14 commits into from
May 15, 2023

Conversation

dejavudwh
Copy link
Contributor

@dejavudwh dejavudwh commented May 14, 2023

拉取/合并请求描述:(PR description)

添加了根据ignore_format.yml的ignore_files属性过滤掉不需要cppcheck的文件。

为什么提交这份PR (why to submit this PR)

#7458

你的解决方案是什么 (what is your solution)

根据file_check.py中原先过滤出的new_file,再次过滤出C/C++文件,筛选之后再进行cppcheck。

#7497 的另一种方法,这个方法可以提高代码复用性,静态代码分析也属于file check,所以功能上也合理。所以这个版本应该会更好一点。

在什么测试环境下测试通过 (what is the test environment)

fork后repo中的github action测试通过。

修改cppcheck会出现错误的文件:

  • case 1:不将文件加入.ignore_format.yml,CI报错
  • case 2:将文件加入.ignore_format.yml的file_path,CI不报错
  • case 3:将文件加入.ignore_format.yml的dir_path,CI不报错

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用formatting等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification

@dejavudwh dejavudwh changed the title Ci filecheck format ignore [CI] Added feature to filter out files to file_check.py that do not require cppcheck May 14, 2023
@dejavudwh dejavudwh force-pushed the CI-filecheck-format-ignore branch from 5d2a179 to d5ca69a Compare May 14, 2023 11:01
@supperthomas
Copy link
Member

supperthomas commented May 14, 2023

尝试了一下,确实报错。但是好像并没有输出哪里报错了,看下图。
图片

实际上报错的地方如下图:

图片

@supperthomas
Copy link
Member

supperthomas commented May 14, 2023

图片

你改的这个函数看起来也没有正确显示cppcheck里面的错误内容。

这个错误内容可以从哪里看到吗?下图是我本地的cppcheck检查

图片

@dejavudwh dejavudwh force-pushed the CI-filecheck-format-ignore branch from f769343 to 5bd39d8 Compare May 14, 2023 14:29
@dejavudwh dejavudwh force-pushed the CI-filecheck-format-ignore branch from 5bd39d8 to d931fbe Compare May 14, 2023 14:33
@dejavudwh
Copy link
Contributor Author

图片

你改的这个函数看起来也没有正确显示cppcheck里面的错误内容。

这个错误内容可以从哪里看到吗?下图是我本地的cppcheck检查

图片

@supperthomas 忘记打stderr了,已经改好了

image

@supperthomas supperthomas added the action github action yml imporve label May 15, 2023
@dejavudwh dejavudwh force-pushed the CI-filecheck-format-ignore branch 7 times, most recently from 03844e4 to 46e5a1f Compare May 15, 2023 15:03
@dejavudwh dejavudwh force-pushed the CI-filecheck-format-ignore branch from 46e5a1f to 4a6bc13 Compare May 15, 2023 15:09
#
# Change Logs:
# Date Author Notes
# 2021-04-01 LiuKang the first version
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

注释改下。

tools/ci/format_ignore.py Show resolved Hide resolved
tools/ci/format_ignore.py Show resolved Hide resolved
@click.argument(
'repo',
nargs=1,
type=click.STRING,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

去掉

- master
on: [push]
# pull_request:
# branches:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

去掉

@@ -1,5 +1,5 @@
#
# Copyright (c) 2006-2022, RT-Thread Development Team
# Copyright (c) 2006-2023, RT-Thread Development Team
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件不用改

@dejavudwh dejavudwh force-pushed the CI-filecheck-format-ignore branch from 02044f3 to 86eb2f1 Compare May 15, 2023 16:21
@dejavudwh dejavudwh requested a review from supperthomas May 15, 2023 16:35
tools/ci/file_check.py Outdated Show resolved Hide resolved
@dejavudwh dejavudwh force-pushed the CI-filecheck-format-ignore branch from 86eb2f1 to 5adfde1 Compare May 15, 2023 16:47
@mysterywolf
Copy link
Member

@supperthomas 涛哥,这个PR你认为没啥问题之后可以直接合并掉

@supperthomas
Copy link
Member

图片

@supperthomas
Copy link
Member

感谢PR。

@supperthomas supperthomas merged commit b1584e9 into RT-Thread:master May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action github action yml imporve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants