[CI] Use --merge-base in check_approval.sh to ensure correct diff scope#7601
Merged
Merged
Conversation
|
Thanks for your contribution! |
Collaborator
Author
|
/skip-ci all |
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 AI Code Review |
2026-04-23 22:03:07
📋 Review 摘要
PR 概述:将 check_approval.sh 中所有 git diff upstream/$BRANCH 替换为 git diff --merge-base upstream/$BRANCH,确保 CI 审批检查仅基于当前分支实际引入的变更进行计算,避免因本地分支未严格 rebase 导致误判。
变更范围:scripts/check_approval.sh(CI 审批脚本)
影响面 Tag:CI
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| ❓ 疑问 | scripts/check_approval.sh |
--merge-base 选项的 Git 版本兼容性 |
❓ 疑问:--merge-base 选项的 Git 版本兼容性
git diff --merge-base 是 Git 2.30.0(2020 年 12 月)引入的新选项。若 CI 环境的 Git 版本低于 2.30,该选项将导致命令失败,进而中断整个审批检查流程。
建议在合并前确认所有 CI Runner 的 Git 版本 ≥ 2.30,或考虑使用如下等价的向下兼容写法:
# 等价写法,兼容 git < 2.30
git diff $(git merge-base HEAD upstream/$BRANCH) -U0 | grep '^\+' | grep -zoE "PD_BUILD_(STATIC_)?OP" || true总体评价
变更思路正确,使用 --merge-base 确实能更准确地界定 PR 引入的变更范围,解决了未严格 rebase 时 diff 范围偏大的问题。建议在合并前确认 CI 环境 Git 版本兼容性以规避潜在风险。
This was referenced Apr 24, 2026
xiaoguoguo626807
pushed a commit
to xiaoguoguo626807/FastDeploy
that referenced
this pull request
May 7, 2026
sunlei1024
pushed a commit
to sunlei1024/FastDeploy
that referenced
this pull request
May 7, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Using
git diff upstream/$BRANCHdirectly may include unrelated changes when the local branch is not strictly rebased, leading to incorrect detection results in CI checks.Switching to
--merge-baseensures that diffs are computed from the common ancestor of the branches, providing a more accurate and stable comparison baseline.Modifications
scripts/check_approval.shto use:git diff --merge-base upstream/$BRANCHUsage or Command
N/A
Accuracy Tests
N/A
Checklist
[FDConfig],[APIServer],[Engine],[Scheduler],[PD Disaggregation],[Executor],[Graph Optimization],[Speculative Decoding],[RL],[Models],[Quantization],[Loader],[OP],[KVCache],[DataProcessor],[BugFix],[Docs],[CI],[Optimization],[Feature],[Benchmark],[Others],[XPU],[HPU],[GCU],[DCU],[Iluvatar],[Metax]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.