-
Notifications
You must be signed in to change notification settings - Fork 245
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.
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
Removing old bitbucket pull request commits #421
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the very late delay. Would appreciate merging with master
to ensure we pick up the latest CI changes 👍
@@ -41,6 +41,12 @@ def grouped_comments(comments) | |||
comments.group_by { |comment| [comment.path, comment.position] } | |||
end | |||
|
|||
def ungrouped_comments(comments) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why this should be in git_formatter
, should directly be in bitbucket_pull_request_formatter
.
|
||
submit_comments(client, additions) | ||
|
||
approve_pull_request(comments.count, additions.count, client) if defined?(approve_pull_request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need to check this? It's defined below.
additions = remove_duplicate_comments(existing, comments) | ||
|
||
remove_comments(client, existing, comments) if existing.any? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be optional. I'm not sure how we'd go about it, maybe a separate PullRequestFormatter
which deletes comments?
Fix #255 for Bitbucket Pull Request format.