From 816ddeeb9ef79cca86310bb9c20a1b804bef5e76 Mon Sep 17 00:00:00 2001 From: Zohar Meir <33152084+zmeir@users.noreply.github.com> Date: Mon, 30 Oct 2023 09:06:51 +0200 Subject: [PATCH] Fix call to `_get_previous_review_comment` Hey @mrT23, I thinks there's a problem with moving this line to after `self.git_provider.publish_comment(pr_comment)`. The reason I originally placed it here is because otherwise, if you run `/review --pr_reviewer.remove_previous_review_comment=true` it will publish your review and then immediately after delete it, because it will look for the previous review comment only after you published your new review - so it will take your new review as the previous one. In order to get the real "previous" review you must collect the comments list before publishing a review, so placing this method call first ensures that. The method `self._get_previous_review_comment()` is a no-op if `pr_reviewer.remove_previous_review_comment=false` so I see no downside in keeping it before `self.git_provider.publish_comment(pr_comment)` Additionally, the check for `if previous_review_comment:` is redundant because it's done internally in `self._remove_previous_review_comment`. I thought it looked cleaner without this extra nesting here, but if you think more verbosity is better I'll keep it. --- pr_agent/tools/pr_reviewer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index d46f24e3..fdb7161e 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -116,9 +116,9 @@ 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() - previous_review_comment = self._get_previous_review_comment() if previous_review_comment: self._remove_previous_review_comment(previous_review_comment) if get_settings().pr_reviewer.inline_code_comments: