Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix bitbucket improve issue #240

Merged
merged 8 commits into from
Sep 10, 2023
Merged

Fix bitbucket improve issue #240

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
67ff505
fix improve, update_changelog and review inline comment
sarbjitsinghgrewal Aug 24, 2023
db6bf41
update readme
sarbjitsinghgrewal Aug 24, 2023
f0dc485
Merge branch 'main' of https://github.com/Codium-ai/pr-agent into fix…
sarbjitsinghgrewal Aug 24, 2023
0e42634
add publish_labels and get_labels functions
sarbjitsinghgrewal Aug 25, 2023
39522ab
fix conflicts
sarbjitsinghgrewal Aug 28, 2023
2aaa722
Merge branch 'main' of https://github.com/Codium-ai/pr-agent into fix…
sarbjitsinghgrewal Aug 29, 2023
970a789
Merge branch 'main' of https://github.com/Codium-ai/pr-agent into fix…
sarbjitsinghgrewal Aug 31, 2023
704c169
Merge branch 'main' of https://github.com/Codium-ai/pr-agent into fix…
sarbjitsinghgrewal Sep 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,12 @@ CodiumAI `PR-Agent` is an open-source tool aiming to help developers review pull
| | | GitHub | Gitlab | Bitbucket | CodeCommit |
|-------|---------------------------------------------|:------:|:------:|:---------:|:----------:|
| TOOLS | Review | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: |
| | ⮑ Inline review | :white_check_mark: | :white_check_mark: | | |
| | ⮑ Inline review | :white_check_mark: | :white_check_mark: | :white_check_mark: |
| | Ask | :white_check_mark: | :white_check_mark: | :white_check_mark: |
| | Auto-Description | :white_check_mark: | :white_check_mark: | | :white_check_mark: |
| | Improve Code | :white_check_mark: | :white_check_mark: | | |
| | ⮑ Extended | :white_check_mark: | :white_check_mark: | | |
| | Reflect and Review | :white_check_mark: | | | |
| | Update CHANGELOG.md | :white_check_mark: | | | |
| | Auto-Description | :white_check_mark: | :white_check_mark: | :white_check_mark: |
| | Improve Code | :white_check_mark: | :white_check_mark: | :white_check_mark: |
| | Reflect and Review | :white_check_mark: | | :white_check_mark: |
| | Update CHANGELOG.md | :white_check_mark: | | :white_check_mark: |
| | | | | | |
| USAGE | CLI | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: |
| | App / webhook | :white_check_mark: | :white_check_mark: | | |
Expand Down
66 changes: 39 additions & 27 deletions pr_agent/git_providers/bitbucket_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from atlassian.bitbucket import Cloud
from starlette_context import context

from ..algo.pr_processing import clip_tokens, find_line_number_of_relevant_line_in_file
from ..config_loader import get_settings
from .git_provider import FilePatchInfo, GitProvider

Expand Down Expand Up @@ -101,12 +102,8 @@ def publish_code_suggestions(self, code_suggestions: list) -> bool:
return False

def is_supported(self, capability: str) -> bool:
if capability in [
"get_issue_comments",
"create_inline_comment",
"publish_inline_comments",
"get_labels",
]:
if capability in ['get_issue_comments', 'publish_inline_comments', 'get_labels']:

return False
return True

Expand Down Expand Up @@ -151,27 +148,43 @@ def remove_initial_comment(self):
except Exception as e:
logging.exception(f"Failed to remove temp comments, error: {e}")

def publish_inline_comment(
self, comment: str, from_line: int, to_line: int, file: str
):
payload = json.dumps(
{
"content": {
"raw": comment,
},
"inline": {"to": from_line, "path": file},
}
)

# funtion to create_inline_comment
def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_file: str):
position, absolute_position = find_line_number_of_relevant_line_in_file(self.get_diff_files(), relevant_file.strip('`'), relevant_line_in_file)
if position == -1:
if get_settings().config.verbosity_level >= 2:
logging.info(f"Could not find position for {relevant_file} {relevant_line_in_file}")
subject_type = "FILE"
else:
subject_type = "LINE"
path = relevant_file.strip()
return dict(body=body, path=path, position=absolute_position) if subject_type == "LINE" else {}


def publish_inline_comment(self, comment: str, from_line: int, file: str):
payload = json.dumps( {
"content": {
"raw": comment,
},
"inline": {
"to": from_line,
"path": file
},
})
response = requests.request(
"POST", self.bitbucket_comment_api_url, data=payload, headers=self.headers
)
return response
Comment on lines 176 to 178
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider using the requests.post method instead of requests.request with 'POST' as a parameter. This would make the code more readable and easier to understand.

Suggested change
"POST", self.bitbucket_comment_api_url, data=payload, headers=self.headers
)
return response
response = requests.post(self.bitbucket_comment_api_url, data=payload, headers=self.headers)


def publish_inline_comments(self, comments: list[dict]):
for comment in comments:
self.publish_inline_comment(
comment["body"], comment["start_line"], comment["line"], comment["path"]
)
self.publish_inline_comment(comment['body'], comment['start_line'], comment['path'])

def publish_bitbucket_inline_comments(self, comments: list[dict]):
for comment in comments:
self.publish_inline_comment(comment['body'],comment['position'], comment['path'])


def get_title(self):
return self.pr.title
Expand Down Expand Up @@ -238,16 +251,15 @@ def _get_pr_file_content(self, remote_link: str):

def get_commit_messages(self):
return "" # not implemented yet


# bitbucket does not support labels
def publish_description(self, pr_title: str, pr_body: str):
pass
def create_inline_comment(
self, body: str, relevant_file: str, relevant_line_in_file: str
):
pass

def publish_labels(self, labels):
# bitbucket does not support labels
def publish_labels(self, pr_types: list):
pass


# bitbucket does not support labels
def get_labels(self):
pass
18 changes: 11 additions & 7 deletions pr_agent/tools/pr_description.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,18 @@ async def run(self):
if get_settings().config.publish_output:
logging.info('Pushing answer...')
if get_settings().pr_description.publish_description_as_comment:
self.git_provider.publish_comment(markdown_text)
self.git_provider.publish_comment(pr_body)
else:
self.git_provider.publish_description(pr_title, pr_body)
if self.git_provider.is_supported("get_labels"):
current_labels = self.git_provider.get_labels()
if current_labels is None:
current_labels = []
self.git_provider.publish_labels(pr_types + current_labels)
# bitbucket does not support publishing PR labels yet
if get_settings().config.git_provider == 'bitbucket':
return
Comment on lines +79 to +80
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider removing the redundant return statement in line 80. If the function doesn't need to return a value, there's no need to include a return statement.

Suggested change
if get_settings().config.git_provider == 'bitbucket':
return
if get_settings().config.git_provider == 'bitbucket':
pass

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't refer explicitly to a Git provider outside the git providers types (use "is_supported")

else:
self.git_provider.publish_description(pr_title, pr_body)
if self.git_provider.is_supported("get_labels"):
current_labels = self.git_provider.get_labels()
if current_labels is None:
current_labels = []
self.git_provider.publish_labels(pr_types + current_labels)
self.git_provider.remove_initial_comment()

return ""
Expand Down
5 changes: 4 additions & 1 deletion pr_agent/tools/pr_reviewer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,10 @@ def _publish_inline_code_comments(self) -> None:
self.git_provider.publish_inline_comment(content, relevant_file, relevant_line_in_file)

if comments:
self.git_provider.publish_inline_comments(comments)
if get_settings().config.git_provider == 'bitbucket':
self.git_provider.publish_bitbucket_inline_comments(comments)
else:
self.git_provider.publish_inline_comments(comments)
Comment on lines +269 to +272
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider refactoring the code to avoid duplicating the publish_inline_comments method. You can create a single method that handles both cases, reducing code redundancy and improving maintainability.

Suggested change
if get_settings().config.git_provider == 'bitbucket':
self.git_provider.publish_bitbucket_inline_comments(comments)
else:
self.git_provider.publish_inline_comments(comments)
self.git_provider.publish_inline_comments(comments, is_bitbucket=get_settings().config.git_provider == 'bitbucket')


def _get_user_answers(self) -> Tuple[str, str]:
"""
Expand Down
2 changes: 1 addition & 1 deletion pr_agent/tools/pr_update_changelog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def __init__(self, pr_url: str, cli_mode=False, args=None):
get_settings().pr_update_changelog_prompt.user)

async def run(self):
assert type(self.git_provider) == GithubProvider, "Currently only Github is supported"
# assert type(self.git_provider) == GithubProvider, "Currently only Github is supported"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider replacing the assertion with a more user-friendly error handling mechanism. Assertions can be turned off globally in the Python interpreter, and they are typically used for debugging, not for handling run-time errors.

Suggested change
# assert type(self.git_provider) == GithubProvider, "Currently only Github is supported"
if not isinstance(self.git_provider, GithubProvider):
raise NotImplementedError("Currently only Github is supported")


logging.info('Updating the changelog...')
if get_settings().config.publish_output:
Expand Down