From c623c3baf4b9915d6ca89a8312aa14f40e1d4ea2 Mon Sep 17 00:00:00 2001 From: zmeir Date: Wed, 1 Nov 2023 21:02:25 +0200 Subject: [PATCH] Added new configurations to prevent too frequent incremental commits on push trigger --- pr_agent/git_providers/git_provider.py | 11 ++++++-- pr_agent/git_providers/github_provider.py | 4 +-- pr_agent/settings/configuration.toml | 6 +++++ pr_agent/tools/pr_reviewer.py | 31 ++++++++++++++++++++--- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 9dee9e3c..3511c2a6 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -190,6 +190,13 @@ class IncrementalPR: def __init__(self, is_incremental: bool = False): self.is_incremental = is_incremental self.commits_range = None - self.first_new_commit_sha = None - self.last_seen_commit_sha = None + self.first_new_commit = None + self.last_seen_commit = None + @property + def first_new_commit_sha(self): + return None if self.first_new_commit is None else self.first_new_commit.sha + + @property + def last_seen_commit_sha(self): + return None if self.last_seen_commit is None else self.last_seen_commit.sha diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 40b0e121..5ed35d04 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -66,10 +66,10 @@ class GithubProvider(GitProvider): first_new_commit_index = None for index in range(len(self.commits) - 1, -1, -1): if self.commits[index].commit.author.date > last_review_time: - self.incremental.first_new_commit_sha = self.commits[index].sha + self.incremental.first_new_commit = self.commits[index] first_new_commit_index = index else: - self.incremental.last_seen_commit_sha = self.commits[index].sha + self.incremental.last_seen_commit = self.commits[index] break return self.commits[first_new_commit_index:] if first_new_commit_index is not None else [] diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index ffe6d39d..06519f0a 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -25,6 +25,9 @@ inline_code_comments = false ask_and_reflect=false automatic_review=true remove_previous_review_comment=false +require_all_thresholds_for_incremental_review=false +minimal_commits_for_incremental_review=0 +minimal_minutes_for_incremental_review=0 extra_instructions = "" [pr_description] # /describe # @@ -105,6 +108,9 @@ push_commands = [ --pr_reviewer.num_code_suggestions=0 \ --pr_reviewer.inline_code_comments=false \ --pr_reviewer.remove_previous_review_comment=true \ + --pr_reviewer.require_all_thresholds_for_incremental_review=false \ + --pr_reviewer.minimal_commits_for_incremental_review=5 \ + --pr_reviewer.minimal_minutes_for_incremental_review=30 \ --pr_reviewer.extra_instructions='' \ """ ] diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 7cf561e2..c13e7edd 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -1,4 +1,5 @@ import copy +import datetime from collections import OrderedDict from typing import List, Tuple @@ -100,9 +101,33 @@ class PRReviewer: if self.is_auto and not get_settings().pr_reviewer.automatic_review: get_logger().info(f'Automatic review is disabled {self.pr_url}') return None - if self.is_auto and self.incremental.is_incremental and not self.incremental.first_new_commit_sha: - get_logger().info(f"Incremental review is enabled for {self.pr_url} but there are no new commits") - return None + if self.incremental.is_incremental: + if self.is_auto and not self.incremental.first_new_commit_sha: + get_logger().info(f"Incremental review is enabled for {self.pr_url} but there are no new commits") + return None + # checking if there are enough commits to start the review + num_new_commits = len(self.incremental.commits_range) + num_commits_threshold = get_settings().pr_reviewer.minimal_commits_for_incremental_review + not_enough_commits = num_new_commits < num_commits_threshold + # checking if the commits are not too recent to start the review + recent_commits_threshold = datetime.datetime.now() - datetime.timedelta( + minutes=get_settings().pr_reviewer.minimal_minutes_for_incremental_review + ) + last_seen_commit_date = ( + self.incremental.last_seen_commit.commit.author.date if self.incremental.last_seen_commit else None + ) + all_commits_too_recent = ( + last_seen_commit_date > recent_commits_threshold if self.incremental.last_seen_commit else False + ) + # check all the thresholds or just one to start the review + condition = any if get_settings().pr_reviewer.require_all_thresholds_for_incremental_review else all + if condition((not_enough_commits, all_commits_too_recent)): + get_logger().info( + f"Incremental review is enabled for {self.pr_url} but didn't pass the threshold check to run:" + f"\n* Number of new commits = {num_new_commits} (threshold is {num_commits_threshold})" + f"\n* Last seen commit date = {last_seen_commit_date} (threshold is {recent_commits_threshold})" + ) + return None get_logger().info(f'Reviewing PR: {self.pr_url} ...')