spuru9 commented on code in PR #28341:
URL: https://github.com/apache/flink/pull/28341#discussion_r3707070288
##########
.github/workflows/community-review.sh:
##########
@@ -243,41 +257,72 @@ process_target_branch_label() {
# 'community-reviewed' is set.
# If one of the labels is set the other is unset.
#
+# Reviews written by someone who worked on the PR do not count towards the
community
+# review tally - neither the author's own reviews, nor those of a co-author. A
request
+# for changes is still honoured, whoever it came from.
+#
# Arguments:
# $1 - GitHub API token for authentication
# $2 - PR number
# $3 - PR reviews
+# $4 - PR author login (excluded from the review tally)
+# $5 - Labels already on the PR
# =============================================================================
process_pr_reviews() {
local token="${1?missing token}"
local pr_number="${2?missing pr number}"
local pr_reviews="${3?missing pr reviews}"
+ local pr_author="${4?missing pr author}"
+ local existing_labels="${5-}"
local communityApproves=0
local requestForChanges=0
local committerApproves=0
local communityReviews=0
local push_permission
+ local pr_coauthors
# replace spaces with new lines so the loop will work
pr_reviews=$(echo "$pr_reviews" | tr ' ' '\n')
# remove unnecessary double quotes
pr_reviews="${pr_reviews//\"/}"
- while IFS=, read -r user state time
+ # A co-author check can only lower the tally, so it can never win the PR a
label it does not
+ # already have. The approve count is a loose upper bound - it only errs
towards checking.
+ local possible_approves
+ possible_approves=$(grep -c ",APPROVED," <<< "$pr_reviews") ||
possible_approves=0
+ local coauthor_lookup_needed=true
+ if [[ "$existing_labels" =~
(^|[[:space:]])"$COMMUNITY_REVIEW_LABEL"($|[[:space:]]) ]] && [[
$possible_approves -lt 2 ]]; then
+ coauthor_lookup_needed=false
+ fi
+
+ while IFS=, read -r user state time first_review_time
do
Review Comment:
It isn't gating a merge. This function picks between community-reviewed and
community-reviewed-LGTM, and LGTM needs 2 approves — so if the PR already has
community-reviewed and there aren't 2 approves, neither label can change and
the co-author lookup is skipped.
This just help in reducing the calls.
I have added a seperate last commit for this so can remove it if its add
unnecessary complexity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
