-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Update detection API add new check document #15848
Conversation
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.
LGTM
paddle/scripts/paddle_build.sh
Outdated
@@ -451,10 +451,10 @@ function assert_api_spec_approvals() { | |||
if [ ${API_CHANGE} ] && [ "${GIT_PR_ID}" != "" ]; then | |||
# NOTE: per_page=10000 should be ok for all cases, a PR review > 10000 is not human readable. | |||
APPROVALS=`curl -H "Authorization: token ${GITHUB_API_TOKEN}" https://api.github.com/repos/PaddlePaddle/Paddle/pulls/${GIT_PR_ID}/reviews?per_page=10000 | \ | |||
python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 2887803` | |||
python ${PADDLE_ROOT}/tools/check_pr_approval.py 2 2887803 35982308` |
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.
I think only API.spec change need PM approval. Other files, like op_desc.h doesn't need PM approval.
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.
I think you're right. I added an if judgment.
74a6f49
4c77996
4edb54d
5a3ba84
Update detection API add new check document