snuyanzin commented on code in PR #28341:
URL: https://github.com/apache/flink/pull/28341#discussion_r3706957047
##########
.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:
not having/pressing `approve` is never a stopper for committer
committer can push directly to GitHub without any approvals, so this code is
mostly confusing and useless tbh... Or did I miss anything?
--
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]
