| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
[Infra] Skip clang-tidy check in PR-CI-Codestyle-Check #66191
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
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
tools/codestyle/clang-tidy.py
Outdated
| 'pip install --no-cache clang-tidy=="15.0.2.1"', shell=True | ||
| ) | ||
| except: | ||
| sys.exit(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这样只是绕过问题吧，并没有解决问题
@GreatV 看看呢？
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
为啥会安装失败呀，是新的python版本没有这个包了吗？如果只是网络问题，就多点几次吧。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
重复点是同样问题，文件数量多时出现
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
还是得看看为啥会安装失败
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不需要知道，这是不暴露给用户的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
开发人员不容易知道是在流水线配置，一般会认为是其他代码里配置
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那先按环境变量修改了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
开发人员不容易知道是在流水线配置，一般会认为是其他代码里配置
所以问题是什么呢？如果你觉得在流水线配置不优雅，就在 tools/codestyle/pre_commit.sh 里加
本地 pre-commit 跑 clang-tidy，CI 上 pre-commit 不跑 clang-tidy 是现在的共识吧？你有更好的解决方案么？
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
都一样是绕过问题
tools/codestyle/clang-tidy.py
Outdated
| @@ -62,6 +62,8 @@ | |||
| import threading | |||
| import traceback | |||
|
|
|||
| import paddle | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不要 import paddle，这个逻辑一行就可以写完
if os.getenv("SKIP_CLANG_TIDY_CHECK", "").lower() in ['y', 'yes', 't', 'true', 'on', '1']:
print("SKIP_CLANG_TIDY_CHECK is set, skip clang-tidy check.", file=sys.stderr)
sys.exit(0)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修改
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except已增加输出日志显示错误信息
tools/codestyle/clang-tidy.py
Outdated
| 'pip install --no-cache clang-tidy=="15.0.2.1"', shell=True | ||
| ) | ||
| except: | ||
| sys.exit(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不建议修改这里，这会导致用户在不同网络环境出现不一致的检查结果，进而导致出现问题很难调试
SigureMo
changed the title
PR Category
Others
PR Types
Others
Description
问题 closes #66158
closes #66265
如果 clang-tidy 安装失败，需要设置退出为0，否则检查不通过