diff --git a/example.config.yaml b/example.config.yaml index 6f79886c..717c460d 100644 --- a/example.config.yaml +++ b/example.config.yaml @@ -93,5 +93,6 @@ repositories: required_linear_history: True required_conversation_resolution: True + minimum-lgtm: 0 # The minimum PR lgtm required before approve the PR set-auto-merge-prs: - main diff --git a/webhook_server_container/config/schema.yaml b/webhook_server_container/config/schema.yaml index 282cc9d8..e0da22a4 100644 --- a/webhook_server_container/config/schema.yaml +++ b/webhook_server_container/config/schema.yaml @@ -173,3 +173,4 @@ properties: type: string conventional-title: string + minimum-lgtm: integer diff --git a/webhook_server_container/libs/github_api.py b/webhook_server_container/libs/github_api.py index a1d9d4f2..acd8638c 100644 --- a/webhook_server_container/libs/github_api.py +++ b/webhook_server_container/libs/github_api.py @@ -29,6 +29,7 @@ from webhook_server_container.libs.config import Config from webhook_server_container.utils.constants import ( ADD_STR, + APPROVE_STR, APPROVED_BY_LABEL_PREFIX, BRANCH_LABEL_PREFIX, BUILD_AND_PUSH_CONTAINER_STR, @@ -50,6 +51,7 @@ HAS_CONFLICTS_LABEL_STR, HOLD_LABEL_STR, IN_PROGRESS_STR, + LABELS_SEPARATOR, LGTM_BY_LABEL_PREFIX, LGTM_STR, NEEDS_REBASE_LABEL_STR, @@ -364,6 +366,7 @@ def _repo_data_from_config(self) -> None: ) self.conventional_title: str = self.config.get_value(value="conventional-title") self.set_auto_merge_prs: list[str] = self.config.get_value(value="set-auto-merge-prs", return_on_none=[]) + self.minimum_lgtm: int = self.config.get_value(value="minimum-lgtm", return_on_none=0) def _get_pull_request(self, number: int | None = None) -> PullRequest: if number: @@ -595,16 +598,19 @@ def add_size_label(self) -> None: self._add_label(label=size_label) def label_by_user_comment( - self, user_requested_label: str, remove: bool, reviewed_user: str, issue_comment_id: int + self, + user_requested_label: str, + remove: bool, + reviewed_user: str, ) -> None: self.logger.debug( f"{self.log_prefix} {DELETE_STR if remove else ADD_STR} " f"label requested by user {reviewed_user}: {user_requested_label}" ) - if user_requested_label == LGTM_STR: + if user_requested_label in (LGTM_STR, APPROVE_STR): self.manage_reviewed_by_label( - review_state=LGTM_STR, + review_state=user_requested_label, action=DELETE_STR if remove else ADD_STR, reviewed_user=reviewed_user, ) @@ -866,26 +872,31 @@ def process_pull_request_webhook_data(self) -> None: if hook_action in ("labeled", "unlabeled"): _check_for_merge: bool = False - _reviewer: str | None = None + _user: str | None = None action_labeled = hook_action == "labeled" labeled = self.hook_data["label"]["name"].lower() + if labeled == CAN_BE_MERGED_STR: return self.logger.info(f"{self.log_prefix} PR {self.pull_request.number} {hook_action} with {labeled}") - if labeled.startswith(APPROVED_BY_LABEL_PREFIX): - _reviewer = labeled.split(APPROVED_BY_LABEL_PREFIX)[-1] - if labeled.startswith(CHANGED_REQUESTED_BY_LABEL_PREFIX): - _reviewer = labeled.split(CHANGED_REQUESTED_BY_LABEL_PREFIX)[-1] + _split_label = labeled.split(LABELS_SEPARATOR, 1) - _approved_output: dict[str, Any] = {"title": "Approved", "summary": "", "text": ""} - if _reviewer in self.all_approvers: - _check_for_merge = True - _approved_output["text"] += f"Approved by {_reviewer}.\n" + if len(_split_label) == 2: + _lable_prefix, _user = _split_label + + if f"{_lable_prefix}{LABELS_SEPARATOR}" in ( + APPROVED_BY_LABEL_PREFIX, + LGTM_BY_LABEL_PREFIX, + CHANGED_REQUESTED_BY_LABEL_PREFIX, + ): + if _user in self.all_reviewers + self.all_approvers: + _check_for_merge = True if self.verified_job and labeled == VERIFIED_LABEL_STR: _check_for_merge = True + if action_labeled: self.set_verify_check_success() else: @@ -933,25 +944,24 @@ def manage_reviewed_by_label(self, review_state: str, action: str, reviewed_user label_prefix: str = "" label_to_remove: str = "" - if reviewed_user in self.all_approvers: - approved_lgtm_label = APPROVED_BY_LABEL_PREFIX - else: - approved_lgtm_label = LGTM_BY_LABEL_PREFIX + if review_state == APPROVE_STR and reviewed_user in self.all_approvers: + label_prefix = APPROVED_BY_LABEL_PREFIX + label_to_remove = f"{CHANGED_REQUESTED_BY_LABEL_PREFIX}{reviewed_user}" - if review_state in ("approved", LGTM_STR): - base_dict = self.hook_data.get("issue", self.hook_data.get("pull_request")) - pr_owner = base_dict["user"]["login"] - if pr_owner == reviewed_user: - self.logger.info(f"{self.log_prefix} PR owner {pr_owner} set /lgtm, not adding label.") - return + elif review_state in ("approved", LGTM_STR): + if base_dict := self.hook_data.get("issue", self.hook_data.get("pull_request")): + pr_owner = base_dict["user"]["login"] + if pr_owner == reviewed_user: + self.logger.info(f"{self.log_prefix} PR owner {pr_owner} set /lgtm, not adding label.") + return _remove_label = f"{CHANGED_REQUESTED_BY_LABEL_PREFIX}{reviewed_user}" - label_prefix = approved_lgtm_label + label_prefix = LGTM_BY_LABEL_PREFIX label_to_remove = _remove_label elif review_state == "changes_requested": label_prefix = CHANGED_REQUESTED_BY_LABEL_PREFIX - _remove_label = approved_lgtm_label + _remove_label = LGTM_BY_LABEL_PREFIX label_to_remove = _remove_label elif review_state == "commented": @@ -1038,8 +1048,6 @@ def _run_pre_commit(self) -> None: return self.set_run_pre_commit_check_failure(output=output) def user_commands(self, command: str, reviewed_user: str, issue_comment_id: int) -> None: - self.create_comment_reaction(issue_comment_id=issue_comment_id, reaction=REACTIONS.ok) - available_commands: list[str] = [ COMMAND_RETEST_STR, COMMAND_CHERRY_PICK_STR, @@ -1072,6 +1080,8 @@ def user_commands(self, command: str, reviewed_user: str, issue_comment_id: int) self.pull_request.create_issue_comment(missing_command_arg_comment_msg) return + self.create_comment_reaction(issue_comment_id=issue_comment_id, reaction=REACTIONS.ok) + if _command == COMMAND_ASSIGN_REVIEWER_STR: self._add_reviewer_by_user_comment(reviewer=_args) @@ -1133,7 +1143,6 @@ def user_commands(self, command: str, reviewed_user: str, issue_comment_id: int) user_requested_label=_command, remove=remove, reviewed_user=reviewed_user, - issue_comment_id=issue_comment_id, ) def cherry_pick(self, target_branch: str, reviewed_user: str = "") -> None: @@ -1283,9 +1292,9 @@ def check_if_can_be_merged(self) -> None: if required_check_failed_failure_output: failure_output += required_check_failed_failure_output - lables_failue_output = self._check_lables_for_can_be_merged(labels=_labels) - if lables_failue_output: - failure_output += lables_failue_output + labels_failure_output = self._check_lables_for_can_be_merged(labels=_labels) + if labels_failure_output: + failure_output += labels_failure_output pr_approvered_failure_output = self._check_if_pr_approved(labels=_labels) if pr_approvered_failure_output: @@ -1504,6 +1513,7 @@ def create_comment_reaction(self, issue_comment_id: int, reaction: str) -> None: def process_opened_or_synchronize_pull_request(self) -> None: prepare_pull_futures: list[Future] = [] + with ThreadPoolExecutor() as executor: prepare_pull_futures.append(executor.submit(self.assign_reviewers)) prepare_pull_futures.append( @@ -2103,7 +2113,7 @@ def _check_lables_for_can_be_merged(self, labels: list[str]) -> str: for _label in labels: if CHANGED_REQUESTED_BY_LABEL_PREFIX.lower() in _label.lower(): - change_request_user = _label.split("-")[-1] + change_request_user = _label.split(LABELS_SEPARATOR)[-1] if change_request_user in self.all_approvers: failure_output += "PR has changed requests from approvers\n" @@ -2118,15 +2128,21 @@ def _check_lables_for_can_be_merged(self, labels: list[str]) -> str: return failure_output def _check_if_pr_approved(self, labels: list[str]) -> str: - self.logger.debug(f"{self.log_prefix} _check_if_pr_approved.") + self.logger.info(f"{self.log_prefix} Check if pull request is approved by pull request labels.") + + error: str = "" approved_by = [] + lgtm_count: int = 0 + + if self.minimum_lgtm: + for _label in labels: + reviewer = _label.split(LABELS_SEPARATOR)[-1] + if LGTM_BY_LABEL_PREFIX.lower() in _label.lower() and reviewer in self.all_reviewers: + lgtm_count += 1 + for _label in labels: if APPROVED_BY_LABEL_PREFIX.lower() in _label.lower(): - approved_user = _label.split("-")[-1] - if self.parent_committer == approved_user: - continue - - approved_by.append(approved_user) + approved_by.append(_label.split(LABELS_SEPARATOR)[-1]) missing_approvers = self.all_approvers.copy() @@ -2143,13 +2159,18 @@ def _check_if_pr_approved(self, labels: list[str]) -> str: break missing_approvers = list(set(missing_approvers)) - if self.parent_committer in missing_approvers: - missing_approvers.remove(self.parent_committer) if missing_approvers: - return f"Missing lgtm/approved from approvers: {', '.join(missing_approvers)}\n" + error += f"Missing approved from approvers: {', '.join(missing_approvers)}\n" - return "" + if lgtm_count < self.minimum_lgtm: + all_reviewers = self.all_reviewers.copy() + if self.parent_committer in all_reviewers: + all_reviewers.pop(all_reviewers.index(self.parent_committer)) + + error += f"Missing lgtm from reviewers. Minimum {self.minimum_lgtm} required. Reviewers: {', '.join(all_reviewers)}.\n" + + return error def _add_reviewer_by_user_comment(self, reviewer: str) -> None: reviewer = reviewer.strip("@") diff --git a/webhook_server_container/utils/constants.py b/webhook_server_container/utils/constants.py index 5dc5c995..6c7ee871 100644 --- a/webhook_server_container/utils/constants.py +++ b/webhook_server_container/utils/constants.py @@ -14,13 +14,15 @@ CONVENTIONAL_TITLE_STR: str = "conventional-title" WIP_STR: str = "wip" LGTM_STR: str = "lgtm" -CHERRY_PICK_LABEL_PREFIX: str = "cherry-pick-" +APPROVE_STR: str = "approve" +LABELS_SEPARATOR: str = "-" +CHERRY_PICK_LABEL_PREFIX: str = f"cherry-pick{LABELS_SEPARATOR}" CHERRY_PICKED_LABEL_PREFIX: str = "CherryPicked" -APPROVED_BY_LABEL_PREFIX: str = "approved-" -LGTM_BY_LABEL_PREFIX: str = f"{LGTM_STR}-" -CHANGED_REQUESTED_BY_LABEL_PREFIX: str = "changes-requested-" -COMMENTED_BY_LABEL_PREFIX: str = "commented-" -BRANCH_LABEL_PREFIX: str = "branch-" +APPROVED_BY_LABEL_PREFIX: str = f"approved{LABELS_SEPARATOR}" +LGTM_BY_LABEL_PREFIX: str = f"{LGTM_STR}{LABELS_SEPARATOR}" +CHANGED_REQUESTED_BY_LABEL_PREFIX: str = f"changes-requested{LABELS_SEPARATOR}" +COMMENTED_BY_LABEL_PREFIX: str = f"commented{LABELS_SEPARATOR}" +BRANCH_LABEL_PREFIX: str = f"branch{LABELS_SEPARATOR}" VERIFIED_LABEL_STR: str = "verified" NEEDS_REBASE_LABEL_STR: str = "needs-rebase" HAS_CONFLICTS_LABEL_STR: str = "has-conflicts" @@ -38,6 +40,7 @@ VERIFIED_LABEL_STR: "0E8A16", WIP_STR: "B60205", LGTM_STR: "0E8A16", + APPROVE_STR: "0E8A16", } STATIC_LABELS_DICT: dict[str, str] = {