From 02570ea797623fc3edecad5fc40d4b829162f7e2 Mon Sep 17 00:00:00 2001 From: zmeir Date: Thu, 26 Oct 2023 11:24:24 +0300 Subject: [PATCH] Remove previous review comment on push event --- pr_agent/git_providers/bitbucket_provider.py | 7 ++++- pr_agent/git_providers/codecommit_provider.py | 3 ++ pr_agent/git_providers/gerrit_provider.py | 3 ++ pr_agent/git_providers/git_provider.py | 4 +++ pr_agent/git_providers/github_provider.py | 28 +++++++++++++------ pr_agent/git_providers/gitlab_provider.py | 8 +++++- pr_agent/git_providers/local_git_provider.py | 3 ++ pr_agent/settings/configuration.toml | 2 ++ pr_agent/tools/pr_reviewer.py | 26 ++++++++++++++++- 9 files changed, 73 insertions(+), 11 deletions(-) diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index fef51794..fe416d5c 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -142,10 +142,15 @@ class BitbucketProvider(GitProvider): def remove_initial_comment(self): try: for comment in self.temp_comments: - self.pr.delete(f"comments/{comment}") + self.remove_comment(comment) except Exception as e: get_logger().exception(f"Failed to remove temp comments, error: {e}") + def remove_comment(self, comment): + try: + self.pr.delete(f"comments/{comment}") + except Exception as e: + get_logger().exception(f"Failed to remove comment, error: {e}") # funtion to create_inline_comment def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): diff --git a/pr_agent/git_providers/codecommit_provider.py b/pr_agent/git_providers/codecommit_provider.py index 4e12f96e..a4836849 100644 --- a/pr_agent/git_providers/codecommit_provider.py +++ b/pr_agent/git_providers/codecommit_provider.py @@ -221,6 +221,9 @@ class CodeCommitProvider(GitProvider): def remove_initial_comment(self): return "" # not implemented yet + def remove_comment(self, comment): + return "" # not implemented yet + def publish_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str): # https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/codecommit/client/post_comment_for_compared_commit.html raise NotImplementedError("CodeCommit provider does not support publishing inline comments yet") diff --git a/pr_agent/git_providers/gerrit_provider.py b/pr_agent/git_providers/gerrit_provider.py index ae017fbd..fd67f5f0 100644 --- a/pr_agent/git_providers/gerrit_provider.py +++ b/pr_agent/git_providers/gerrit_provider.py @@ -396,5 +396,8 @@ class GerritProvider(GitProvider): # shutil.rmtree(self.repo_path) pass + def remove_comment(self, comment): + pass + def get_pr_branch(self): return self.repo.head diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index c6740467..6f986cf0 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -71,6 +71,10 @@ class GitProvider(ABC): def remove_initial_comment(self): pass + @abstractmethod + def remove_comment(self, comment): + pass + @abstractmethod def get_languages(self): pass diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 7a47fbf5..88fb3863 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -50,7 +50,7 @@ class GithubProvider(GitProvider): def get_incremental_commits(self): self.commits = list(self.pr.get_commits()) - self.get_previous_review() + self.previous_review = self.get_previous_review(full=True, incremental=True) if self.previous_review: self.incremental.commits_range = self.get_commit_range() # Get all files changed during the commit range @@ -73,13 +73,19 @@ class GithubProvider(GitProvider): break return self.commits[first_new_commit_index:] - def get_previous_review(self): - self.previous_review = None - self.comments = list(self.pr.get_issue_comments()) + def get_previous_review(self, *, full: bool, incremental: bool): + if not (full or incremental): + raise ValueError("At least one of full or incremental must be True") + if not getattr(self, "comments", None): + self.comments = list(self.pr.get_issue_comments()) + prefixes = [] + if full: + prefixes.append("## PR Analysis") + if incremental: + prefixes.append("## Incremental PR Review") for index in range(len(self.comments) - 1, -1, -1): - if self.comments[index].body.startswith("## PR Analysis") or self.comments[index].body.startswith("## Incremental PR Review"): - self.previous_review = self.comments[index] - break + if any(self.comments[index].body.startswith(prefix) for prefix in prefixes): + return self.comments[index] def get_files(self): if self.incremental.is_incremental and self.file_set: @@ -218,10 +224,16 @@ class GithubProvider(GitProvider): try: for comment in getattr(self.pr, 'comments_list', []): if comment.is_temporary: - comment.delete() + self.remove_comment(comment) except Exception as e: get_logger().exception(f"Failed to remove initial comment, error: {e}") + def remove_comment(self, comment): + try: + comment.delete() + except Exception as e: + get_logger().exception(f"Failed to remove comment, error: {e}") + def get_title(self): return self.pr.title diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 0d09e622..d36fecf9 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -287,10 +287,16 @@ class GitLabProvider(GitProvider): def remove_initial_comment(self): try: for comment in self.temp_comments: - comment.delete() + self.remove_comment(comment) except Exception as e: get_logger().exception(f"Failed to remove temp comments, error: {e}") + def remove_comment(self, comment): + try: + comment.delete() + except Exception as e: + get_logger().exception(f"Failed to remove comment, error: {e}") + def get_title(self): return self.mr.title diff --git a/pr_agent/git_providers/local_git_provider.py b/pr_agent/git_providers/local_git_provider.py index 5fa9f0be..0ef11413 100644 --- a/pr_agent/git_providers/local_git_provider.py +++ b/pr_agent/git_providers/local_git_provider.py @@ -140,6 +140,9 @@ class LocalGitProvider(GitProvider): def remove_initial_comment(self): pass # Not applicable to the local git provider, but required by the interface + def remove_comment(self, comment): + pass # Not applicable to the local git provider, but required by the interface + def get_languages(self): """ Calculate percentage of languages in repository. Used for hunk prioritisation. diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 32fcfb83..3fdcf93c 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -24,6 +24,7 @@ num_code_suggestions=4 inline_code_comments = false ask_and_reflect=false automatic_review=true +remove_previous_review_comment=false extra_instructions = "" [pr_description] # /describe # @@ -100,6 +101,7 @@ push_commands = [ --pr_reviewer.require_estimate_effort_to_review=false \ --pr_reviewer.num_code_suggestions=0 \ --pr_reviewer.inline_code_comments=false \ + --pr_reviewer.remove_previous_review_comment=true \ --pr_reviewer.extra_instructions='' \ """ ] diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 0cc695e3..b13b5e17 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -114,9 +114,10 @@ class PRReviewer: if get_settings().config.publish_output: get_logger().info('Pushing PR review...') + previous_review_comment = self._get_previous_review_comment() self.git_provider.publish_comment(pr_comment) self.git_provider.remove_initial_comment() - + self._remove_previous_review_comment(previous_review_comment) if get_settings().pr_reviewer.inline_code_comments: get_logger().info('Pushing inline code comments...') self._publish_inline_code_comments() @@ -318,3 +319,26 @@ class PRReviewer: break return question_str, answer_str + + def _get_previous_review_comment(self): + """ + Get the previous review comment if it exists. + """ + try: + if get_settings().pr_reviewer.remove_previous_review_comment and hasattr(self.git_provider, "get_previous_review"): + return self.git_provider.get_previous_review( + full=not self.incremental.is_incremental, + incremental=self.incremental.is_incremental, + ) + except Exception as e: + get_logger().exception(f"Failed to get previous review comment, error: {e}") + + def _remove_previous_review_comment(self, comment): + """ + Get the previous review comment if it exists. + """ + try: + if get_settings().pr_reviewer.remove_previous_review_comment and comment: + self.git_provider.remove_comment(comment) + except Exception as e: + get_logger().exception(f"Failed to remove previous review comment, error: {e}")