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.

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

Exclude bot's comment from "Time to first response" #121

Merged
merged 3 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ The metrics that are measured are:
| Time to answer | (Discussions only) The time between when a discussion is created and when it is answered. |
| Time in label | The time between when a label has a specific label applied to an issue/pull request/discussion and when it is removed. This requires the LABELS_TO_MEASURE env variable to be set. |

*For pull requests, these metrics exclude the time the PR was in draft mode.
*For pull requests, these metrics exclude the time the PR was in draft mode.
*For Issue and pull requests, issue/pull request author's own comments and comments by bots are excluded.

This action was developed by the GitHub OSPO for our own use and developed in a way that we could open source it that it might be useful to you as well! If you want to know more about how we use it, reach out in an issue in this repository.

Expand Down
30 changes: 30 additions & 0 deletions test_time_to_first_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,36 @@ def test_measure_time_to_first_response_ignore_issue_owners_comment(self):
# Check the results
self.assertEqual(result, expected_result)

def test_measure_time_to_first_response_ignore_bot(self):
"""Test that measure_time_to_first_response ignore bot's comment."""
# Set up the mock GitHub issues
mock_issue1 = MagicMock()
mock_issue1.comments = 2
mock_issue1.created_at = "2023-01-01T00:00:00Z"

# Set up the mock GitHub issue comments
mock_comment1 = MagicMock()
mock_comment1.user.type = "Bot"
mock_comment1.created_at = datetime.fromisoformat("2023-01-02T00:00:00Z")
mock_issue1.issue.comments.return_value = [mock_comment1]

# Set up the mock GitHub pull request comments
mock_pr_comment1 = MagicMock()
mock_pr_comment1.user.type = "Bot"
mock_pr_comment1.submitted_at = datetime.fromisoformat("2023-01-03T00:00:00Z")
mock_pr_comment2 = MagicMock()
mock_pr_comment2.user.type = "User"
mock_pr_comment2.submitted_at = datetime.fromisoformat("2023-01-04T00:00:00Z") # first response
mock_pull_request = MagicMock()
mock_pull_request.reviews.return_value = [mock_pr_comment1, mock_pr_comment2]

# Call the function
result = measure_time_to_first_response(mock_issue1, None, mock_pull_request, None)
expected_result = timedelta(days=3)

# Check the results
self.assertEqual(result, expected_result)


class TestGetAverageTimeToFirstResponse(unittest.TestCase):
"""Test the get_average_time_to_first_response function."""
Expand Down
32 changes: 22 additions & 10 deletions time_to_first_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@ def measure_time_to_first_response(
number=20, sort="created", direction="asc"
) # type: ignore
for comment in comments:
if comment.user.login in ignore_users:
continue
if comment.user.login == issue.issue.user.login:
continue
if ready_for_review_at and comment.created_at < ready_for_review_at:
if ignore_comment(issue.issue.user, comment.user, ignore_users, comment.created_at, ready_for_review_at):
continue
first_comment_time = comment.created_at
break
Expand All @@ -71,11 +67,8 @@ def measure_time_to_first_response(
if pull_request:
review_comments = pull_request.reviews(number=50) # type: ignore
for review_comment in review_comments:
if review_comment.user.login in ignore_users:
continue
if review_comment.user.login == issue.issue.user.login:
continue
if ready_for_review_at and review_comment.submitted_at < ready_for_review_at:
if ignore_comment(issue.issue.user, review_comment.user, ignore_users,
review_comment.submitted_at, ready_for_review_at):
continue
first_review_comment_time = review_comment.submitted_at
break
Expand Down Expand Up @@ -109,6 +102,25 @@ def measure_time_to_first_response(
return None


def ignore_comment(
issue_user: github3.users.User,
comment_user: github3.users.User,
ignore_users: List[str],
comment_created_at: datetime,
ready_for_review_at: Union[datetime, None],
) -> bool:
"""Check if a comment should be ignored."""
return (
# ignore comments by IGNORE_USERS
comment_user.login in ignore_users
# ignore comments by bots
or comment_user.type == "Bot"
Copy link
Member

Choose a reason for hiding this comment

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

Were you able to test this one out? I was worried by the documentations description:

A string representing the type of User account this. In all cases should be “User”

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not read the documentation, but I have confirmed that a "bot" is returned when the API is called.

For example.

% gh api \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /users/codesee-maps\[bot\]
{
  "login": "codesee-maps[bot]",
  ...
  "type": "Bot",
  ...
}

As a side note, I hope the api documentation clearly states this. However, the Examples only mentions "User".

https://docs.github.com/en/rest/users/users?apiVersion=2022-11-28#get-a-user

# ignore comments by the issue creator
or comment_user.login == issue_user.login
# ignore comments created before the issue was ready for review
or (ready_for_review_at and comment_created_at < ready_for_review_at))


def get_average_time_to_first_response(
issues: List[IssueWithMetrics],
) -> Union[timedelta, None]:
Expand Down