From 9867b1b67d9166629f1ff7ab7e331c854f41a3ee Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Tue, 13 Feb 2024 10:15:42 +0100 Subject: [PATCH 01/18] First stab at figuring out the number of very active mentors in a project. --- most_active_mentors.py | 149 ++++++++++++++++++++++++++++++++++++ test_most_active_mentors.py | 54 +++++++++++++ 2 files changed, 203 insertions(+) create mode 100644 most_active_mentors.py create mode 100644 test_most_active_mentors.py diff --git a/most_active_mentors.py b/most_active_mentors.py new file mode 100644 index 0000000..8af2142 --- /dev/null +++ b/most_active_mentors.py @@ -0,0 +1,149 @@ +"""A module for measuring the number of very active mentors + +This module provides functions for measuring the number of active mentors on a project. + +This is measured by number of PR comments. We are working under the assumption that PR +comments are left in good faith to move contributors further instead of nitpicking and +dis-couraging them. + +Open questions: + - should there be an option to limit this to certain users, e.g. core maintainers? + - should there be a limit to how many comments per PR we consider to avoid having + the statistic dominated by contested PRs? + - should this metric count consecutive comments coming from the same user as only + one to avoid people unnessesarily splitting their comments to game the metric? + - instead of PR comments should we count PRs on which a username was seen as commenter? + +Functions: + collect_response_usernames( + issue: Union[github3.issues.Issue, None], + discussion: Union[dict, None], + pull_request: Union[github3.pulls.PullRequest, None], + max_comments_to_evaluate, + ) -> ____________ + Collect the number of responses per username for single item. Take only top n + comments (max_comments_to_evaluate) into consideration. + get_number_of_active_reviewers( + mentors: List [mentors with metrics) + ) -> int active_number + Count the number of mentors active at least n times + +""" +from datetime import datetime, timedelta +from typing import List, Union + +import github3 +import numpy + +from classes import IssueWithMetrics + + +def count_comments_per_user( + issue: Union[github3.issues.Issue, None], # type: ignore + discussion: Union[dict, None] = None, + pull_request: Union[github3.pulls.PullRequest, None] = None, + ready_for_review_at: Union[datetime, None] = None, + ignore_users: List[str] = None, + max_comments_to_eval = 20, +) -> dict: + """Count the number of times a user was seen commenting on a single item. + + Args: + issue (Union[github3.issues.Issue, None]): A GitHub issue. + discussion (Union[dict, None]): A GitHub discussion. + pull_request (Union[github3.pulls.PullRequest, None]): A GitHub pull request. + ignore_users (List[str]): A list of GitHub usernames to ignore. + max_comments_to_eval: Maximum number of comments per item to look at. + + Returns: + dict: A dictionary of usernames seen and number of comments they left. + + """ + if ignore_users is None: + ignore_users = [] + mentor_count = {} + + # Get the first comments + if issue: + comments = issue.issue.comments( + number=max_comments_to_eval, sort="created", direction="asc" + ) # type: ignore + for comment in comments: + if ignore_comment( + issue.issue.user, + comment.user, + ignore_users, + comment.created_at, + ready_for_review_at, + ): + continue + # increase the number of comments left by current user by 1 + if (comment.user.login in mentor_count): + mentor_count[comment.user.login] += 1 + else: + mentor_count[comment.user.login] = 1 + + # Check if the issue is actually a pull request + # so we may also get the first review comment time + if pull_request: + review_comments = pull_request.reviews(number=max_comments_to_eval) # type: ignore + for review_comment in review_comments: + if ignore_comment( + issue.issue.user, + review_comment.user, + ignore_users, + review_comment.submitted_at, + ready_for_review_at, + ): + continue + + # increase the number of comments left by current user by 1 + if (review_comment.user.login in mentor_count): + mentor_count[review_comment.user.login] += 1 + else: + mentor_count[review_comment.user.login] = 1 + + return mentor_count + + +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" + # 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_mentor_count( + mentor_activity: dict, + cutoff: int +) -> int: + """ Calculate the number of active mentors on the project. + + Args: + mentor_activity (dict: A dictionary with usernames to count of comments left. + cutoff (int: the minimum number of comments a user has to leave to count as active mentor.) + + Returns: + int: Number of active mentors + + """ + active_mentor_count = 0 + for mentor, count in mentor_activity.items(): + if (count >= cutoff): + active_mentor_count += 1 + + return active_mentor_count + diff --git a/test_most_active_mentors.py b/test_most_active_mentors.py new file mode 100644 index 0000000..76501e9 --- /dev/null +++ b/test_most_active_mentors.py @@ -0,0 +1,54 @@ +"""A module containing unit tests for the most_active_mentors module. + +This module contains unit tests for the count_comments_per_user and +get_mentor_count functions in the most_active_mentors module. +The tests use mock GitHub issues and comments to test the functions' behavior. + +Classes: + TestCountCommentsPerUser: A class to test the count_comments_per_user function. + TestGetMentorCount: A class to test the + get_mentor_count function. + +""" +import unittest +from datetime import datetime, timedelta +from unittest.mock import MagicMock + +from classes import IssueWithMetrics +from most_active_mentors import ( + count_comments_per_user, + get_mentor_count, +) + + +class TestCountCommentsPerUser(unittest.TestCase): + """Test the count_comments_per_user function.""" + + def test_count_comments_per_user(self): + """Test that count_comments_per_user correctly counts user comments. + + This test mocks the GitHub connection and issue comments, and checks that + count_comments_per_user correctly considers user comments for counting. + + """ + # Set up the mock GitHub issues + mock_issue1 = MagicMock() + mock_issue1.comments = 2 + mock_issue1.issue.user.login = "issue_owner" + mock_issue1.created_at = "2023-01-01T00:00:00Z" + + # Set up 21 mock GitHub issue comments - only 20 should be counted + mock_issue1.issue.comments.return_value = [] + for i in range(22): + mock_comment1 = MagicMock() + mock_comment1.user.login = "very_active_user" + mock_comment1.created_at = datetime.fromisoformat(f"2023-01-02T{i:02d}:00:00Z") + mock_issue1.issue.comments.return_value.append(mock_comment1) + + # Call the function + result = count_comments_per_user(mock_issue1) + expected_result = {"very_active_user": 20} + + # Check the results + self.assertEqual(result, expected_result) + From a144ed865c2b1beda6862eb04e53dff2b9ef05df Mon Sep 17 00:00:00 2001 From: Drost-Fromm Date: Thu, 21 Mar 2024 10:19:23 +0100 Subject: [PATCH 02/18] Fix format and linter errors. --- most_active_mentors.py | 65 ++++++++++++++++++++----------------- test_most_active_mentors.py | 14 ++++---- 2 files changed, 41 insertions(+), 38 deletions(-) mode change 100644 => 100755 most_active_mentors.py mode change 100644 => 100755 test_most_active_mentors.py diff --git a/most_active_mentors.py b/most_active_mentors.py old mode 100644 new mode 100755 index 8af2142..e5f2e78 --- a/most_active_mentors.py +++ b/most_active_mentors.py @@ -1,18 +1,22 @@ """A module for measuring the number of very active mentors -This module provides functions for measuring the number of active mentors on a project. +This module provides functions for measuring the number of active mentors on a +project. -This is measured by number of PR comments. We are working under the assumption that PR -comments are left in good faith to move contributors further instead of nitpicking and -dis-couraging them. +This is measured by number of PR comments. We are working under the assumption +that PR comments are left in good faith to move contributors further instead of +nitpicking and discouraging them. Open questions: - - should there be an option to limit this to certain users, e.g. core maintainers? - - should there be a limit to how many comments per PR we consider to avoid having - the statistic dominated by contested PRs? - - should this metric count consecutive comments coming from the same user as only - one to avoid people unnessesarily splitting their comments to game the metric? - - instead of PR comments should we count PRs on which a username was seen as commenter? + - should there be an option to limit this to certain users, e.g. core + maintainers? + - should there be a limit to how many comments per PR we consider to avoid + having the statistic dominated by contested PRs? + - should this metric count consecutive comments coming from the same user as + only one to avoid people unnessesarily splitting their comments to game the + metric? + - instead of PR comments should we count PRs on which a username was seen as + commenter? Functions: collect_response_usernames( @@ -21,39 +25,38 @@ pull_request: Union[github3.pulls.PullRequest, None], max_comments_to_evaluate, ) -> ____________ - Collect the number of responses per username for single item. Take only top n - comments (max_comments_to_evaluate) into consideration. + Collect the number of responses per username for single item. Take only + top n comments (max_comments_to_evaluate) into consideration. get_number_of_active_reviewers( mentors: List [mentors with metrics) ) -> int active_number Count the number of mentors active at least n times """ -from datetime import datetime, timedelta +from datetime import datetime from typing import List, Union import github3 -import numpy - -from classes import IssueWithMetrics def count_comments_per_user( issue: Union[github3.issues.Issue, None], # type: ignore - discussion: Union[dict, None] = None, pull_request: Union[github3.pulls.PullRequest, None] = None, ready_for_review_at: Union[datetime, None] = None, ignore_users: List[str] = None, - max_comments_to_eval = 20, + max_comments_to_eval=20, + heavily_involved=3, ) -> dict: """Count the number of times a user was seen commenting on a single item. Args: issue (Union[github3.issues.Issue, None]): A GitHub issue. - discussion (Union[dict, None]): A GitHub discussion. - pull_request (Union[github3.pulls.PullRequest, None]): A GitHub pull request. - ignore_users (List[str]): A list of GitHub usernames to ignore. + pull_request (Union[github3.pulls.PullRequest, None]): A GitHub pull + request. ignore_users (List[str]): A list of GitHub usernames to + ignore. max_comments_to_eval: Maximum number of comments per item to look at. + heavily_involved: Maximum number of comments to count for one + user per issue. Returns: dict: A dictionary of usernames seen and number of comments they left. @@ -78,15 +81,17 @@ def count_comments_per_user( ): continue # increase the number of comments left by current user by 1 - if (comment.user.login in mentor_count): - mentor_count[comment.user.login] += 1 + if comment.user.login in mentor_count: + if mentor_count[comment.user.login] < heavily_involved: + mentor_count[comment.user.login] += 1 else: mentor_count[comment.user.login] = 1 # Check if the issue is actually a pull request # so we may also get the first review comment time if pull_request: - review_comments = pull_request.reviews(number=max_comments_to_eval) # type: ignore + review_comments = pull_request.reviews(number=max_comments_to_eval) + # type: ignore for review_comment in review_comments: if ignore_comment( issue.issue.user, @@ -98,7 +103,7 @@ def count_comments_per_user( continue # increase the number of comments left by current user by 1 - if (review_comment.user.login in mentor_count): + if review_comment.user.login in mentor_count: mentor_count[review_comment.user.login] += 1 else: mentor_count[review_comment.user.login] = 1 @@ -133,17 +138,17 @@ def get_mentor_count( """ Calculate the number of active mentors on the project. Args: - mentor_activity (dict: A dictionary with usernames to count of comments left. - cutoff (int: the minimum number of comments a user has to leave to count as active mentor.) + mentor_activity (dict: A dictionary with usernames to count of comments + left. cutoff (int: the minimum number of comments a user has to leave + to count as active mentor.) Returns: int: Number of active mentors """ active_mentor_count = 0 - for mentor, count in mentor_activity.items(): - if (count >= cutoff): + for count in mentor_activity.values(): + if count >= cutoff: active_mentor_count += 1 return active_mentor_count - diff --git a/test_most_active_mentors.py b/test_most_active_mentors.py old mode 100644 new mode 100755 index 76501e9..4e2bcce --- a/test_most_active_mentors.py +++ b/test_most_active_mentors.py @@ -5,19 +5,17 @@ The tests use mock GitHub issues and comments to test the functions' behavior. Classes: - TestCountCommentsPerUser: A class to test the count_comments_per_user function. + TestCountCommentsPerUser: A class testing count_comments_per_user. TestGetMentorCount: A class to test the get_mentor_count function. """ import unittest -from datetime import datetime, timedelta +from datetime import datetime from unittest.mock import MagicMock -from classes import IssueWithMetrics from most_active_mentors import ( count_comments_per_user, - get_mentor_count, ) @@ -27,8 +25,9 @@ class TestCountCommentsPerUser(unittest.TestCase): def test_count_comments_per_user(self): """Test that count_comments_per_user correctly counts user comments. - This test mocks the GitHub connection and issue comments, and checks that - count_comments_per_user correctly considers user comments for counting. + This test mocks the GitHub connection and issue comments, and checks + that count_comments_per_user correctly considers user comments for + counting. """ # Set up the mock GitHub issues @@ -47,8 +46,7 @@ def test_count_comments_per_user(self): # Call the function result = count_comments_per_user(mock_issue1) - expected_result = {"very_active_user": 20} + expected_result = {"very_active_user": 3} # Check the results self.assertEqual(result, expected_result) - From ca1368066176686b73b8fb46c37fb561bdfbc827 Mon Sep 17 00:00:00 2001 From: Drost-Fromm Date: Tue, 26 Mar 2024 09:09:25 +0100 Subject: [PATCH 03/18] Add flake to requirements. --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index c1d2d5f..8e9f6e0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,4 @@ +flake8==7.0.0 github3.py==4.0.1 numpy==1.26.4 python-dotenv==1.0.1 From 0adb1e1946be49213574f48ba50832b1136e0f33 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Tue, 26 Mar 2024 19:40:10 +0100 Subject: [PATCH 04/18] Adds call to mentor counter and output in markdown. This adds the call to mentor counter and displays the results in markdown including first tests for this functionality. --- classes.py | 3 +++ issue_metrics.py | 24 +++++++++++++++++++++--- markdown_writer.py | 8 +++++++- most_active_mentors.py | 19 +++++++++++++++---- test_issue_metrics.py | 2 +- test_markdown_writer.py | 11 ++++++++++- test_most_active_mentors.py | 21 +++++++++++++++++++++ 7 files changed, 78 insertions(+), 10 deletions(-) diff --git a/classes.py b/classes.py index 3867f26..6a2ccce 100644 --- a/classes.py +++ b/classes.py @@ -19,6 +19,7 @@ class IssueWithMetrics: time_to_answer (timedelta, optional): The time it took to answer the discussions in the issue. label_metrics (dict, optional): A dictionary containing the label metrics + mentor_activity (dict, optional): A dictionary containing active mentors """ @@ -31,6 +32,7 @@ def __init__( time_to_close=None, time_to_answer=None, labels_metrics=None, + mentor_activity=None, ): self.title = title self.html_url = html_url @@ -39,3 +41,4 @@ def __init__( self.time_to_close = time_to_close self.time_to_answer = time_to_answer self.label_metrics = labels_metrics + self.mentor_activity = mentor_activity diff --git a/issue_metrics.py b/issue_metrics.py index bd4d040..d56ef1e 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -40,7 +40,11 @@ from time_to_merge import measure_time_to_merge from time_to_ready_for_review import get_time_to_ready_for_review -GITHUB_BASE_URL = "https://github.com" +from most_active_mentors import ( + count_comments_per_user, + get_mentor_count +) +from config import get_env_vars def search_issues( @@ -158,10 +162,15 @@ def get_per_issue_metrics( None, None, None, + None, ) issue_with_metrics.time_to_first_response = measure_time_to_first_response( None, issue, ignore_users ) + issue_with_metrics.mentor_activity = count_comments_per_user( + None, issue, ignore_users + # TODO review arguments max_comments_to_eval, heavily_involved + ) issue_with_metrics.time_to_answer = measure_time_to_answer(issue) if issue["closedAt"]: issue_with_metrics.time_to_close = measure_time_to_close(None, issue) @@ -188,6 +197,10 @@ def get_per_issue_metrics( issue_with_metrics.time_to_first_response = measure_time_to_first_response( issue, None, pull_request, ready_for_review_at, ignore_users ) + issue_with_metrics.mentor_activity = count_comments_per_user( + issue, None, pull_request, ready_for_review_at, ignore_users + ) + # TODO review arguments max_comments_to_eval, heavily_involved if labels: issue_with_metrics.label_metrics = get_label_metrics(issue, labels) if issue.state == "closed": # type: ignore @@ -259,6 +272,7 @@ def main(): token, env_vars.ghe, ) + min_mentor_count = 10 # Get the repository owner and name from the search query owner = get_owner(search_query) @@ -283,13 +297,13 @@ def main(): issues = get_discussions(token, search_query) if len(issues) <= 0: print("No discussions found") - write_to_markdown(None, None, None, None, None, None, None) + write_to_markdown(None, None, None, None, None, None, None, None) return else: issues = search_issues(search_query, github_connection) if len(issues) <= 0: print("No issues found") - write_to_markdown(None, None, None, None, None, None, None) + write_to_markdown(None, None, None, None, None, None, None, None) return # Get all the metrics @@ -307,6 +321,8 @@ def main(): stats_time_to_answer = get_stats_time_to_answer(issues_with_metrics) + num_mentor_count = get_mentor_count(issues_with_metrics, min_mentor_comments) + # Get stats describing the time in label for each label and store it in a dictionary # where the key is the label and the value is the average time stats_time_in_labels = get_stats_time_in_labels(issues_with_metrics, labels) @@ -320,6 +336,7 @@ def main(): stats_time_in_labels, num_issues_open, num_issues_closed, + num_mentor_count, search_query, ) write_to_markdown( @@ -330,6 +347,7 @@ def main(): stats_time_in_labels, num_issues_open, num_issues_closed, + num_mentor_count, labels, search_query, ) diff --git a/markdown_writer.py b/markdown_writer.py index ba49622..0b587ae 100644 --- a/markdown_writer.py +++ b/markdown_writer.py @@ -13,6 +13,7 @@ average_time_to_answer: timedelta, num_issues_opened: int, num_issues_closed: int, + num_mentor_count: int, file: file object = None ) -> None: Write the issues with metrics to a markdown file. @@ -79,6 +80,7 @@ def write_to_markdown( average_time_in_labels: Union[dict, None], num_issues_opened: Union[int, None], num_issues_closed: Union[int, None], + num_mentor_count: Union[int, None], labels=None, search_query=None, hide_label_metrics=False, @@ -95,7 +97,8 @@ def write_to_markdown( file (file object, optional): The file object to write to. If not provided, a file named "issue_metrics.md" will be created. num_issues_opened (int): The Number of items that remain opened. - num_issues_closed (int): The number of issues that were closed. + num_issues_closed (int): The number of issues that were closedi. + num_mentor_count (int): The number of very active commentors. labels (List[str]): A list of the labels that are used in the issues. search_query (str): The search query used to find the issues. hide_label_metrics (bool): Represents whether the user has chosen to hide label metrics in the output @@ -127,6 +130,7 @@ def write_to_markdown( average_time_in_labels, num_issues_opened, num_issues_closed, + num_mentor_count, labels, columns, file, @@ -184,6 +188,7 @@ def write_overall_metrics_tables( stats_time_in_labels, num_issues_opened, num_issues_closed, + num_mentor_count, labels, columns, file, @@ -246,4 +251,5 @@ def write_overall_metrics_tables( file.write("| --- | ---: |\n") file.write(f"| Number of items that remain open | {num_issues_opened} |\n") file.write(f"| Number of items closed | {num_issues_closed} |\n") + file.write(f"| Number of most active mentors | {num_mentor_count} |\n") file.write(f"| Total number of items created | {len(issues_with_metrics)} |\n\n") diff --git a/most_active_mentors.py b/most_active_mentors.py index e5f2e78..e16892d 100755 --- a/most_active_mentors.py +++ b/most_active_mentors.py @@ -36,11 +36,15 @@ from datetime import datetime from typing import List, Union +from collections import Counter + import github3 +from classes import IssueWithMetrics def count_comments_per_user( issue: Union[github3.issues.Issue, None], # type: ignore + discussion: Union[dict, None] = None, pull_request: Union[github3.pulls.PullRequest, None] = None, ready_for_review_at: Union[datetime, None] = None, ignore_users: List[str] = None, @@ -132,22 +136,29 @@ def ignore_comment( def get_mentor_count( - mentor_activity: dict, + issues_with_metrics: List[IssueWithMetrics], cutoff: int ) -> int: """ Calculate the number of active mentors on the project. Args: - mentor_activity (dict: A dictionary with usernames to count of comments - left. cutoff (int: the minimum number of comments a user has to leave + issues_with_metrics (List[IssueWithMetrics]): A list of issues w/ + metrics + cutoff (int: the minimum number of comments a user has to leave to count as active mentor.) Returns: int: Number of active mentors """ + + mentor_count = Counter({}) + for issueWithMetrics in issues_with_metrics: + current_counter = Counter(issueWithMetrics.mentor_activity) + mentor_count = mentor_count + current_counter + active_mentor_count = 0 - for count in mentor_activity.values(): + for count in mentor_count.values(): if count >= cutoff: active_mentor_count += 1 diff --git a/test_issue_metrics.py b/test_issue_metrics.py index 34b2e38..00c5730 100644 --- a/test_issue_metrics.py +++ b/test_issue_metrics.py @@ -231,7 +231,7 @@ def test_main_no_issues_found( # Call main and check that it writes 'No issues found' issue_metrics.main() mock_write_to_markdown.assert_called_once_with( - None, None, None, None, None, None, None + None, None, None, None, None, None, None, None ) diff --git a/test_markdown_writer.py b/test_markdown_writer.py index 00b585c..053dc0a 100644 --- a/test_markdown_writer.py +++ b/test_markdown_writer.py @@ -78,6 +78,7 @@ def test_write_to_markdown(self): num_issues_opened = 2 num_issues_closed = 1 + num_mentor_count = 5 # Call the function write_to_markdown( @@ -88,6 +89,7 @@ def test_write_to_markdown(self): average_time_in_labels=time_in_labels, num_issues_opened=num_issues_opened, num_issues_closed=num_issues_closed, + num_mentor_count=num_mentor_count, labels=["bug"], search_query="is:issue is:open label:bug", ) @@ -108,6 +110,7 @@ def test_write_to_markdown(self): "| --- | ---: |\n" "| Number of items that remain open | 2 |\n" "| Number of items closed | 1 |\n" + "| Number of most active mentors | 5 |\n" "| Total number of items created | 2 |\n\n" "| Title | URL | Author | Time to first response | Time to close |" " Time to answer | Time spent in bug |\n" @@ -175,6 +178,7 @@ def test_write_to_markdown_with_vertical_bar_in_title(self): num_issues_opened = 2 num_issues_closed = 1 + num_mentor_count = 5 # Call the function write_to_markdown( @@ -185,6 +189,7 @@ def test_write_to_markdown_with_vertical_bar_in_title(self): average_time_in_labels=average_time_in_labels, num_issues_opened=num_issues_opened, num_issues_closed=num_issues_closed, + num_mentor_count=num_mentor_count, labels=["bug"], ) @@ -204,6 +209,7 @@ def test_write_to_markdown_with_vertical_bar_in_title(self): "| --- | ---: |\n" "| Number of items that remain open | 2 |\n" "| Number of items closed | 1 |\n" + "| Number of most active mentors | 5 |\n" "| Total number of items created | 2 |\n\n" "| Title | URL | Author | Time to first response | Time to close |" " Time to answer | Time spent in bug |\n" @@ -221,7 +227,7 @@ def test_write_to_markdown_no_issues(self): """Test that write_to_markdown writes the correct markdown file when no issues are found.""" # Call the function with no issues with patch("builtins.open", mock_open()) as mock_open_file: - write_to_markdown(None, None, None, None, None, None, None) + write_to_markdown(None, None, None, None, None, None, None, None) # Check that the file was written correctly expected_output = [ @@ -292,6 +298,7 @@ def test_writes_markdown_file_with_non_hidden_columns_only(self): } num_issues_opened = 2 num_issues_closed = 1 + num_mentor_count = 5 # Call the function write_to_markdown( @@ -302,6 +309,7 @@ def test_writes_markdown_file_with_non_hidden_columns_only(self): average_time_in_labels=average_time_in_labels, num_issues_opened=num_issues_opened, num_issues_closed=num_issues_closed, + num_mentor_count=num_mentor_count, labels=["label1"], search_query="repo:user/repo is:issue", hide_label_metrics=True, @@ -316,6 +324,7 @@ def test_writes_markdown_file_with_non_hidden_columns_only(self): "| --- | ---: |\n" "| Number of items that remain open | 2 |\n" "| Number of items closed | 1 |\n" + "| Number of most active mentors | 5 |\n" "| Total number of items created | 2 |\n\n" "| Title | URL | Author |\n" "| --- | --- | --- |\n" diff --git a/test_most_active_mentors.py b/test_most_active_mentors.py index 4e2bcce..5420844 100755 --- a/test_most_active_mentors.py +++ b/test_most_active_mentors.py @@ -14,8 +14,10 @@ from datetime import datetime from unittest.mock import MagicMock +from classes import IssueWithMetrics from most_active_mentors import ( count_comments_per_user, + get_mentor_count, ) @@ -50,3 +52,22 @@ def test_count_comments_per_user(self): # Check the results self.assertEqual(result, expected_result) + + def test_get_mentor_count(self): + """ Test that get_mentor_count correctly counts comments per user. + + """ + mentor_activity = {"sue": 15, "bob": 10} + + # Create moc data + issues_with_metrics = [ + IssueWithMetrics("Issue 1", "https://github.com/user/repo/issues/1", + "alice", None, mentor_activity=mentor_activity), + IssueWithMetrics("Issue 2", "https://github.com/user/repo/issues/2", + "bob", None, mentor_activity=mentor_activity), + ] + + # Call the function and check the result + result = get_mentor_count(issues_with_metrics, 2) + expected_result = 2 + self.assertEqual(result, expected_result) From f041169ce7875bb7765b060057f1c12de6c731b4 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 27 Mar 2024 20:12:33 +0100 Subject: [PATCH 05/18] Make mentor counting configurable. This adds two configuration options: One to enable mentor counting and one for configuring how many comments a user needs to leave in discussions, PRs. and issues to be counted as an active mentor. --- README.md | 2 ++ config.py | 23 ++++++++++++++++++++++- issue_metrics.py | 7 +++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index f18668c..3a5571a 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,8 @@ This action can be configured to authenticate with GitHub App Installation or Pe | `HIDE_TIME_TO_CLOSE` | False | False | If set to `true`, the time to close will not be displayed in the generated Markdown file. | | `HIDE_TIME_TO_FIRST_RESPONSE` | False | False | If set to `true`, the time to first response will not be displayed in the generated Markdown file. | | `IGNORE_USERS` | False | False | A comma separated list of users to ignore when calculating metrics. (ie. `IGNORE_USERS: 'user1,user2'`). To ignore bots, append `[bot]` to the user (ie. `IGNORE_USERS: 'github-actions[bot]'`) | +| `ENABLE_MENTOR_COUNT` | False | False | If set to 'TRUE' count number of comments users left on discussions, issues and PRs and display number of active mentors | +| `MIN_MENTOR_COMMENTS` | False | 10 | Minimum number of comments to count as a mentor | | `LABELS_TO_MEASURE` | False | `""` | A comma separated list of labels to measure how much time the label is applied. If not provided, no labels durations will be measured. Not compatible with discussions at this time. | | `SEARCH_QUERY` | True | `""` | The query by which you can filter issues/PRs which must contain a `repo:`, `org:`, `owner:`, or a `user:` entry. For discussions, include `type:discussions` in the query. | diff --git a/config.py b/config.py index e7d87be..6a84af8 100644 --- a/config.py +++ b/config.py @@ -34,6 +34,8 @@ class EnvVars: hide_time_to_first_response (bool): If true, the time to first response metric is hidden in the output ignore_users (List[str]): List of usernames to ignore when calculating metrics labels_to_measure (List[str]): List of labels to measure how much time the lable is applied + enable_mentor_count (str): If set to TRUE, compute number of mentors + min_mentor_comments (str): If set, defines the minimum number of comments for mentors search_query (str): Search query used to filter issues/prs/discussions on GitHub """ @@ -65,6 +67,8 @@ def __init__( self.hide_time_to_answer = hide_time_to_answer self.hide_time_to_close = hide_time_to_close self.hide_time_to_first_response = hide_time_to_first_response + self.enable_mentor_count = enable_mentor_count + self.min_mentor_comments = min_mentor_comments self.search_query = search_query def __repr__(self): @@ -82,10 +86,11 @@ def __repr__(self): f"{self.hide_time_to_first_response}," f"{self.ignore_users}," f"{self.labels_to_measure}," + f"{self.enable_mentor_count}," + f"{self.min_mentor_comments}," f"{self.search_query})" ) - def get_bool_env_var(env_var_name: str) -> bool: """Get a boolean environment variable. @@ -161,11 +166,21 @@ def get_env_vars(test: bool = False) -> EnvVars: ignore_users_list = ignore_users.split(",") # Hidden columns +<<<<<<< HEAD hide_author = get_bool_env_var("HIDE_AUTHOR") hide_label_metrics = get_bool_env_var("HIDE_LABEL_METRICS") hide_time_to_answer = get_bool_env_var("HIDE_TIME_TO_ANSWER") hide_time_to_close = get_bool_env_var("HIDE_TIME_TO_CLOSE") hide_time_to_first_response = get_bool_env_var("HIDE_TIME_TO_FIRST_RESPONSE") +======= + hide_author = os.getenv("HIDE_AUTHOR") + hide_time_to_first_response = os.getenv("HIDE_TIME_TO_FIRST_RESPONSE") + hide_time_to_close = os.getenv("HIDE_TIME_TO_CLOSE") + hide_time_to_answer = os.getenv("HIDE_TIME_TO_ANSWER") + hide_label_metrics = os.getenv("HIDE_LABEL_METRICS") + enable_mentor_count = os.getenv("ENABLE_MENTOR_COUNT", "FALSE") + min_mentor_comments = os.getenv("MIN_MENTOR_COMMENTS", "10") +>>>>>>> e5f7987 (Make mentor counting configurable.) return EnvVars( gh_app_id, @@ -176,9 +191,15 @@ def get_env_vars(test: bool = False) -> EnvVars: hide_author, hide_label_metrics, hide_time_to_answer, +<<<<<<< HEAD hide_time_to_close, hide_time_to_first_response, ignore_users_list, labels_to_measure_list, search_query, +======= + hide_label_metrics, + enable_mentor_count, + min_mentor_comments +>>>>>>> e5f7987 (Make mentor counting configurable.) ) diff --git a/issue_metrics.py b/issue_metrics.py index d56ef1e..0edcf33 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -263,6 +263,7 @@ def main(): search_query = env_vars.search_query token = env_vars.gh_token ignore_users = env_vars.ignore_users + enable_mentor_count = env_vars.enable_mentor_count # Auth to GitHub.com github_connection = auth_to_github( @@ -272,7 +273,7 @@ def main(): token, env_vars.ghe, ) - min_mentor_count = 10 + min_mentor_count = int(env_vars.min_mentor_comments) # Get the repository owner and name from the search query owner = get_owner(search_query) @@ -321,7 +322,9 @@ def main(): stats_time_to_answer = get_stats_time_to_answer(issues_with_metrics) - num_mentor_count = get_mentor_count(issues_with_metrics, min_mentor_comments) + num_mentor_count = 0 + if enable_mentor_count == "TRUE": + num_mentor_count = get_mentor_count(issues_with_metrics, min_mentor_comments) # Get stats describing the time in label for each label and store it in a dictionary # where the key is the label and the value is the average time From 0837e49cb689aae7656e0cdbbce124b036c5556e Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Thu, 28 Mar 2024 16:56:32 +0100 Subject: [PATCH 06/18] Adds mentor counting to json output and adds missing config. This adds mentor counting output to json format. In addition this change makes max number of comments to evaluate configurable as well as the cutoff for heavily involved mentors. --- README.md | 2 ++ config.py | 23 +++++++++++------------ issue_metrics.py | 18 ++++++++++++------ json_writer.py | 3 +++ most_active_mentors.py | 22 ++++++++++++++++++++-- test_json_writer.py | 6 ++++++ test_most_active_mentors.py | 10 ++++++---- 7 files changed, 60 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 3a5571a..23ca5ef 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,8 @@ This action can be configured to authenticate with GitHub App Installation or Pe | `IGNORE_USERS` | False | False | A comma separated list of users to ignore when calculating metrics. (ie. `IGNORE_USERS: 'user1,user2'`). To ignore bots, append `[bot]` to the user (ie. `IGNORE_USERS: 'github-actions[bot]'`) | | `ENABLE_MENTOR_COUNT` | False | False | If set to 'TRUE' count number of comments users left on discussions, issues and PRs and display number of active mentors | | `MIN_MENTOR_COMMENTS` | False | 10 | Minimum number of comments to count as a mentor | +| `MAX_COMMENTS_EVAL` | False | 20 | Maximum number of comments per thread to evaluate for mentor stats | +| `HEAVILY_INVOLVED_CUTOFF` | False | 3 | Cutoff after which a mentor's comments in one issue are no longer counted against their total score | | `LABELS_TO_MEASURE` | False | `""` | A comma separated list of labels to measure how much time the label is applied. If not provided, no labels durations will be measured. Not compatible with discussions at this time. | | `SEARCH_QUERY` | True | `""` | The query by which you can filter issues/PRs which must contain a `repo:`, `org:`, `owner:`, or a `user:` entry. For discussions, include `type:discussions` in the query. | diff --git a/config.py b/config.py index 6a84af8..69157b1 100644 --- a/config.py +++ b/config.py @@ -36,6 +36,8 @@ class EnvVars: labels_to_measure (List[str]): List of labels to measure how much time the lable is applied enable_mentor_count (str): If set to TRUE, compute number of mentors min_mentor_comments (str): If set, defines the minimum number of comments for mentors + max_comments_eval (str): If set, defines the maximum number of comments to look at for mentor evaluation + heavily_involved_cutoff (str): If set, defines the cutoff after which heavily involved commentors in search_query (str): Search query used to filter issues/prs/discussions on GitHub """ @@ -58,6 +60,7 @@ def __init__( self.gh_app_id = gh_app_id self.gh_app_installation_id = gh_app_installation_id self.gh_app_private_key_bytes = gh_app_private_key_bytes + self.search_query = search_query self.gh_token = gh_token self.ghe = ghe self.ignore_users = ignore_user @@ -69,6 +72,8 @@ def __init__( self.hide_time_to_first_response = hide_time_to_first_response self.enable_mentor_count = enable_mentor_count self.min_mentor_comments = min_mentor_comments + self.max_comments_eval = max_comments_eval + self.heavily_involved_cutoff = heavily_involved_cutoff self.search_query = search_query def __repr__(self): @@ -88,6 +93,8 @@ def __repr__(self): f"{self.labels_to_measure}," f"{self.enable_mentor_count}," f"{self.min_mentor_comments}," + f"{self.max_comments_eval}," + f"{self.heavily_involved_cutoff}," f"{self.search_query})" ) @@ -166,21 +173,15 @@ def get_env_vars(test: bool = False) -> EnvVars: ignore_users_list = ignore_users.split(",") # Hidden columns -<<<<<<< HEAD hide_author = get_bool_env_var("HIDE_AUTHOR") hide_label_metrics = get_bool_env_var("HIDE_LABEL_METRICS") hide_time_to_answer = get_bool_env_var("HIDE_TIME_TO_ANSWER") hide_time_to_close = get_bool_env_var("HIDE_TIME_TO_CLOSE") hide_time_to_first_response = get_bool_env_var("HIDE_TIME_TO_FIRST_RESPONSE") -======= - hide_author = os.getenv("HIDE_AUTHOR") - hide_time_to_first_response = os.getenv("HIDE_TIME_TO_FIRST_RESPONSE") - hide_time_to_close = os.getenv("HIDE_TIME_TO_CLOSE") - hide_time_to_answer = os.getenv("HIDE_TIME_TO_ANSWER") - hide_label_metrics = os.getenv("HIDE_LABEL_METRICS") enable_mentor_count = os.getenv("ENABLE_MENTOR_COUNT", "FALSE") min_mentor_comments = os.getenv("MIN_MENTOR_COMMENTS", "10") ->>>>>>> e5f7987 (Make mentor counting configurable.) + max_comments_eval = os.getenv("MAX_COMMENTS_EVAL", "20") + heavily_involved_cutoff = os.getenv("HEAVILY_INVOLVED_CUTOFF", "3") return EnvVars( gh_app_id, @@ -191,15 +192,13 @@ def get_env_vars(test: bool = False) -> EnvVars: hide_author, hide_label_metrics, hide_time_to_answer, -<<<<<<< HEAD hide_time_to_close, hide_time_to_first_response, ignore_users_list, labels_to_measure_list, search_query, -======= - hide_label_metrics, enable_mentor_count, min_mentor_comments ->>>>>>> e5f7987 (Make mentor counting configurable.) + max_comments_eval, + heavily_involved_cutoff ) diff --git a/issue_metrics.py b/issue_metrics.py index 0edcf33..3a8ae12 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -130,6 +130,8 @@ def get_per_issue_metrics( discussions: bool = False, labels: Union[List[str], None] = None, ignore_users: Union[List[str], None] = None, + max_comments_to_eval: int = 20, + heavily_involved: int = 3 ) -> tuple[List, int, int]: """ Calculate the metrics for each issue/pr/discussion in a list provided. @@ -168,8 +170,8 @@ def get_per_issue_metrics( None, issue, ignore_users ) issue_with_metrics.mentor_activity = count_comments_per_user( - None, issue, ignore_users - # TODO review arguments max_comments_to_eval, heavily_involved + None, issue, ignore_users, None, None, + max_comments_to_eval, heavily_involved ) issue_with_metrics.time_to_answer = measure_time_to_answer(issue) if issue["closedAt"]: @@ -198,9 +200,9 @@ def get_per_issue_metrics( issue, None, pull_request, ready_for_review_at, ignore_users ) issue_with_metrics.mentor_activity = count_comments_per_user( - issue, None, pull_request, ready_for_review_at, ignore_users + issue, None, pull_request, ready_for_review_at, ignore_users, + max_comments_to_eval, heavily_involved ) - # TODO review arguments max_comments_to_eval, heavily_involved if labels: issue_with_metrics.label_metrics = get_label_metrics(issue, labels) if issue.state == "closed": # type: ignore @@ -263,7 +265,6 @@ def main(): search_query = env_vars.search_query token = env_vars.gh_token ignore_users = env_vars.ignore_users - enable_mentor_count = env_vars.enable_mentor_count # Auth to GitHub.com github_connection = auth_to_github( @@ -273,7 +274,10 @@ def main(): token, env_vars.ghe, ) + enable_mentor_count = env_vars.enable_mentor_count min_mentor_count = int(env_vars.min_mentor_comments) + max_comments_eval = int(env_vars.max_comments_eval) + heavily_involved_cutoff = int(env_vars.heavily_involved_cutoff) # Get the repository owner and name from the search query owner = get_owner(search_query) @@ -313,6 +317,8 @@ def main(): discussions="type:discussions" in search_query, labels=labels, ignore_users=ignore_users, + max_comments_to_eval=max_comments_eval, + heavily_involved=heavily_involved_cutoff, ) stats_time_to_first_response = get_stats_time_to_first_response(issues_with_metrics) @@ -324,7 +330,7 @@ def main(): num_mentor_count = 0 if enable_mentor_count == "TRUE": - num_mentor_count = get_mentor_count(issues_with_metrics, min_mentor_comments) + num_mentor_count = get_mentor_count(issues_with_metrics, min_mentor_count) # Get stats describing the time in label for each label and store it in a dictionary # where the key is the label and the value is the average time diff --git a/json_writer.py b/json_writer.py index 1955ef3..1b40e01 100644 --- a/json_writer.py +++ b/json_writer.py @@ -30,6 +30,7 @@ def write_to_json( stats_time_in_labels: Union[dict[str, dict[str, timedelta]], None], num_issues_opened: Union[int, None], num_issues_closed: Union[int, None], + num_mentor_count: Union[int, None], search_query: str, ) -> str: """ @@ -42,6 +43,7 @@ def write_to_json( "average_time_to_answer": "1 day, 0:00:00", "num_items_opened": 2, "num_items_closed": 1, + "num_mentor_count": 5, "total_item_count": 2, "issues": [ { @@ -129,6 +131,7 @@ def write_to_json( "90_percentile_time_in_labels": p90_time_in_labels, "num_items_opened": num_issues_opened, "num_items_closed": num_issues_closed, + "num_mentor_count": num_mentor_count, "total_item_count": len(issues_with_metrics), } diff --git a/most_active_mentors.py b/most_active_mentors.py index e16892d..4a5dfb1 100755 --- a/most_active_mentors.py +++ b/most_active_mentors.py @@ -42,6 +42,7 @@ from classes import IssueWithMetrics + def count_comments_per_user( issue: Union[github3.issues.Issue, None], # type: ignore discussion: Union[dict, None] = None, @@ -112,6 +113,23 @@ def count_comments_per_user( else: mentor_count[review_comment.user.login] = 1 + if discussion and len(discussion["comments"]["nodes"]) > 0: + for comment in discussion["comments"]["nodes"]: + if ignore_comment( + comment.user, + comment.user, + ignore_users, + comment.submitted_at, + comment.ready_for_review_at + ): + continue + + # increase the number of comments left by current user by 1 + if comment.user.login in mentor_count: + mentor_count[comment.user.login] += 1 + else: + mentor_count[comment.user.login] = 1 + return mentor_count @@ -153,8 +171,8 @@ def get_mentor_count( """ mentor_count = Counter({}) - for issueWithMetrics in issues_with_metrics: - current_counter = Counter(issueWithMetrics.mentor_activity) + for issue_with_metrics in issues_with_metrics: + current_counter = Counter(issue_with_metrics.mentor_activity) mentor_count = mentor_count + current_counter active_mentor_count = 0 diff --git a/test_json_writer.py b/test_json_writer.py index b7b18ac..fe1fe1b 100644 --- a/test_json_writer.py +++ b/test_json_writer.py @@ -61,6 +61,7 @@ def test_write_to_json(self): } num_issues_opened = 2 num_issues_closed = 1 + num_mentor_count = 5 expected_output = { "average_time_to_first_response": "2 days, 12:00:00", @@ -77,6 +78,7 @@ def test_write_to_json(self): "90_percentile_time_in_labels": {"bug": "1 day, 16:24:12"}, "num_items_opened": 2, "num_items_closed": 1, + "num_mentor_count": 5, "total_item_count": 2, "issues": [ { @@ -111,6 +113,7 @@ def test_write_to_json(self): stats_time_in_labels=stats_time_in_labels, num_issues_opened=num_issues_opened, num_issues_closed=num_issues_closed, + num_mentor_count=num_mentor_count, search_query="is:issue repo:owner/repo", ), json.dumps(expected_output), @@ -149,6 +152,7 @@ def test_write_to_json_with_no_response(self): } num_issues_opened = 2 num_issues_closed = 0 + num_mentor_count = 5 expected_output = { "average_time_to_first_response": "None", @@ -165,6 +169,7 @@ def test_write_to_json_with_no_response(self): "90_percentile_time_in_labels": {}, "num_items_opened": 2, "num_items_closed": 0, + "num_mentor_count": 5, "total_item_count": 2, "issues": [ { @@ -199,6 +204,7 @@ def test_write_to_json_with_no_response(self): stats_time_in_labels=stats_time_in_labels, num_issues_opened=num_issues_opened, num_issues_closed=num_issues_closed, + num_mentor_count=num_mentor_count, search_query="is:issue repo:owner/repo", ), json.dumps(expected_output), diff --git a/test_most_active_mentors.py b/test_most_active_mentors.py index 5420844..550b988 100755 --- a/test_most_active_mentors.py +++ b/test_most_active_mentors.py @@ -61,10 +61,12 @@ def test_get_mentor_count(self): # Create moc data issues_with_metrics = [ - IssueWithMetrics("Issue 1", "https://github.com/user/repo/issues/1", - "alice", None, mentor_activity=mentor_activity), - IssueWithMetrics("Issue 2", "https://github.com/user/repo/issues/2", - "bob", None, mentor_activity=mentor_activity), + IssueWithMetrics( + "Issue 1", "https://github.com/user/repo/issues/1", + "alice", None, mentor_activity=mentor_activity), + IssueWithMetrics( + "Issue 2", "https://github.com/user/repo/issues/2", + "bob", None, mentor_activity=mentor_activity), ] # Call the function and check the result From 2c1ebc1041742267fa0e172e010e791bca172c44 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Thu, 28 Mar 2024 20:00:28 +0100 Subject: [PATCH 07/18] Fix merge conflicts. --- config.py | 10 +++++++--- test_config.py | 12 ++++++++++++ test_issue_metrics.py | 2 +- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/config.py b/config.py index 69157b1..e9b7718 100644 --- a/config.py +++ b/config.py @@ -55,6 +55,10 @@ def __init__( hide_time_to_first_response: bool, ignore_user: List[str], labels_to_measure: List[str], + enable_mentor_count: str, + min_mentor_comments: str, + max_comments_eval: str, + heavily_involved_cutoff: str , search_query: str, ): self.gh_app_id = gh_app_id @@ -196,9 +200,9 @@ def get_env_vars(test: bool = False) -> EnvVars: hide_time_to_first_response, ignore_users_list, labels_to_measure_list, - search_query, enable_mentor_count, - min_mentor_comments + min_mentor_comments, max_comments_eval, - heavily_involved_cutoff + heavily_involved_cutoff, + search_query, ) diff --git a/test_config.py b/test_config.py index e4a00bb..ef5fba7 100644 --- a/test_config.py +++ b/test_config.py @@ -118,6 +118,10 @@ def test_get_env_vars_with_github_app(self): False, [], [], + "FALSE", + "10", + "20", + "3", SEARCH_QUERY, ) result = get_env_vars(True) @@ -157,6 +161,10 @@ def test_get_env_vars_with_token(self): False, [], [], + "FALSE", + "10", + "20", + "3", SEARCH_QUERY, ) result = get_env_vars(True) @@ -195,6 +203,10 @@ def test_get_env_vars_optional_values(self): True, [], ["waiting-for-review", "waiting-for-manager"], + "FALSE", + 10, + 20, + 3, SEARCH_QUERY, ) result = get_env_vars(True) diff --git a/test_issue_metrics.py b/test_issue_metrics.py index 00c5730..d192157 100644 --- a/test_issue_metrics.py +++ b/test_issue_metrics.py @@ -123,7 +123,7 @@ def test_get_env_vars_missing_query(self): # Call the function and check that it raises a ValueError with self.assertRaises(ValueError): - get_env_vars() + get_env_vars(test=True) class TestMain(unittest.TestCase): From 1a0109353a4e801d09c8b106963ba5232ebd6276 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Thu, 28 Mar 2024 20:05:05 +0100 Subject: [PATCH 08/18] Fix linting errors. --- config.py | 3 ++- issue_metrics.py | 7 +------ most_active_mentors.py | 4 +--- test_most_active_mentors.py | 5 +---- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/config.py b/config.py index e9b7718..df4f06a 100644 --- a/config.py +++ b/config.py @@ -58,7 +58,7 @@ def __init__( enable_mentor_count: str, min_mentor_comments: str, max_comments_eval: str, - heavily_involved_cutoff: str , + heavily_involved_cutoff: str, search_query: str, ): self.gh_app_id = gh_app_id @@ -102,6 +102,7 @@ def __repr__(self): f"{self.search_query})" ) + def get_bool_env_var(env_var_name: str) -> bool: """Get a boolean environment variable. diff --git a/issue_metrics.py b/issue_metrics.py index 3a8ae12..f3d28a0 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -31,6 +31,7 @@ from json_writer import write_to_json from labels import get_label_metrics, get_stats_time_in_labels from markdown_writer import write_to_markdown +from most_active_mentors import count_comments_per_user, get_mentor_count from time_to_answer import get_stats_time_to_answer, measure_time_to_answer from time_to_close import get_stats_time_to_close, measure_time_to_close from time_to_first_response import ( @@ -40,12 +41,6 @@ from time_to_merge import measure_time_to_merge from time_to_ready_for_review import get_time_to_ready_for_review -from most_active_mentors import ( - count_comments_per_user, - get_mentor_count -) -from config import get_env_vars - def search_issues( search_query: str, github_connection: github3.GitHub diff --git a/most_active_mentors.py b/most_active_mentors.py index 4a5dfb1..5b9acd0 100755 --- a/most_active_mentors.py +++ b/most_active_mentors.py @@ -33,13 +33,11 @@ Count the number of mentors active at least n times """ +from collections import Counter from datetime import datetime from typing import List, Union -from collections import Counter - import github3 - from classes import IssueWithMetrics diff --git a/test_most_active_mentors.py b/test_most_active_mentors.py index 550b988..7c5defd 100755 --- a/test_most_active_mentors.py +++ b/test_most_active_mentors.py @@ -15,10 +15,7 @@ from unittest.mock import MagicMock from classes import IssueWithMetrics -from most_active_mentors import ( - count_comments_per_user, - get_mentor_count, -) +from most_active_mentors import count_comments_per_user, get_mentor_count class TestCountCommentsPerUser(unittest.TestCase): From ddb16e27ae45ab8ef182a2483cad9317e0373c74 Mon Sep 17 00:00:00 2001 From: Zack Koppert Date: Tue, 2 Apr 2024 15:56:28 -0700 Subject: [PATCH 09/18] fix: linting fixes Signed-off-by: Zack Koppert --- issue_metrics.py | 20 +++++++++++++++----- most_active_mentors.py | 20 +++++++++----------- test_most_active_mentors.py | 26 ++++++++++++++++++-------- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/issue_metrics.py b/issue_metrics.py index f3d28a0..8cd9834 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -126,7 +126,7 @@ def get_per_issue_metrics( labels: Union[List[str], None] = None, ignore_users: Union[List[str], None] = None, max_comments_to_eval: int = 20, - heavily_involved: int = 3 + heavily_involved: int = 3, ) -> tuple[List, int, int]: """ Calculate the metrics for each issue/pr/discussion in a list provided. @@ -165,8 +165,13 @@ def get_per_issue_metrics( None, issue, ignore_users ) issue_with_metrics.mentor_activity = count_comments_per_user( - None, issue, ignore_users, None, None, - max_comments_to_eval, heavily_involved + None, + issue, + ignore_users, + None, + None, + max_comments_to_eval, + heavily_involved, ) issue_with_metrics.time_to_answer = measure_time_to_answer(issue) if issue["closedAt"]: @@ -195,8 +200,13 @@ def get_per_issue_metrics( issue, None, pull_request, ready_for_review_at, ignore_users ) issue_with_metrics.mentor_activity = count_comments_per_user( - issue, None, pull_request, ready_for_review_at, ignore_users, - max_comments_to_eval, heavily_involved + issue, + None, + pull_request, + ready_for_review_at, + ignore_users, + max_comments_to_eval, + heavily_involved, ) if labels: issue_with_metrics.label_metrics = get_label_metrics(issue, labels) diff --git a/most_active_mentors.py b/most_active_mentors.py index 5b9acd0..99bdfae 100755 --- a/most_active_mentors.py +++ b/most_active_mentors.py @@ -33,9 +33,10 @@ Count the number of mentors active at least n times """ + from collections import Counter from datetime import datetime -from typing import List, Union +from typing import Dict, List, Union import github3 from classes import IssueWithMetrics @@ -46,7 +47,7 @@ def count_comments_per_user( discussion: Union[dict, None] = None, pull_request: Union[github3.pulls.PullRequest, None] = None, ready_for_review_at: Union[datetime, None] = None, - ignore_users: List[str] = None, + ignore_users: List[str] | None = None, max_comments_to_eval=20, heavily_involved=3, ) -> dict: @@ -67,7 +68,7 @@ def count_comments_per_user( """ if ignore_users is None: ignore_users = [] - mentor_count = {} + mentor_count: Dict[str, int] = {} # Get the first comments if issue: @@ -118,7 +119,7 @@ def count_comments_per_user( comment.user, ignore_users, comment.submitted_at, - comment.ready_for_review_at + comment.ready_for_review_at, ): continue @@ -139,7 +140,7 @@ def ignore_comment( ready_for_review_at: Union[datetime, None], ) -> bool: """Check if a comment should be ignored.""" - return ( + return bool( # ignore comments by IGNORE_USERS comment_user.login in ignore_users # ignore comments by bots @@ -151,11 +152,8 @@ def ignore_comment( ) -def get_mentor_count( - issues_with_metrics: List[IssueWithMetrics], - cutoff: int -) -> int: - """ Calculate the number of active mentors on the project. +def get_mentor_count(issues_with_metrics: List[IssueWithMetrics], cutoff: int) -> int: + """Calculate the number of active mentors on the project. Args: issues_with_metrics (List[IssueWithMetrics]): A list of issues w/ @@ -168,7 +166,7 @@ def get_mentor_count( """ - mentor_count = Counter({}) + mentor_count: Counter[str] = Counter({}) for issue_with_metrics in issues_with_metrics: current_counter = Counter(issue_with_metrics.mentor_activity) mentor_count = mentor_count + current_counter diff --git a/test_most_active_mentors.py b/test_most_active_mentors.py index 7c5defd..15edb95 100755 --- a/test_most_active_mentors.py +++ b/test_most_active_mentors.py @@ -10,6 +10,7 @@ get_mentor_count function. """ + import unittest from datetime import datetime from unittest.mock import MagicMock @@ -40,7 +41,10 @@ def test_count_comments_per_user(self): for i in range(22): mock_comment1 = MagicMock() mock_comment1.user.login = "very_active_user" - mock_comment1.created_at = datetime.fromisoformat(f"2023-01-02T{i:02d}:00:00Z") + mock_comment1.created_at = datetime.fromisoformat( + f"2023-01-02T{i:02d}:00:00Z" + ) + # pylint: disable=maybe-no-member mock_issue1.issue.comments.return_value.append(mock_comment1) # Call the function @@ -51,19 +55,25 @@ def test_count_comments_per_user(self): self.assertEqual(result, expected_result) def test_get_mentor_count(self): - """ Test that get_mentor_count correctly counts comments per user. - - """ + """Test that get_mentor_count correctly counts comments per user.""" mentor_activity = {"sue": 15, "bob": 10} # Create moc data issues_with_metrics = [ IssueWithMetrics( - "Issue 1", "https://github.com/user/repo/issues/1", - "alice", None, mentor_activity=mentor_activity), + "Issue 1", + "https://github.com/user/repo/issues/1", + "alice", + None, + mentor_activity=mentor_activity, + ), IssueWithMetrics( - "Issue 2", "https://github.com/user/repo/issues/2", - "bob", None, mentor_activity=mentor_activity), + "Issue 2", + "https://github.com/user/repo/issues/2", + "bob", + None, + mentor_activity=mentor_activity, + ), ] # Call the function and check the result From 8f7613b3219f5cbdfbb14620d9ff7053e3e1c72f Mon Sep 17 00:00:00 2001 From: Zack Koppert Date: Tue, 2 Apr 2024 16:04:56 -0700 Subject: [PATCH 10/18] 8 is reasonable number of attrs Signed-off-by: Zack Koppert --- classes.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/classes.py b/classes.py index 6a2ccce..e1e6684 100644 --- a/classes.py +++ b/classes.py @@ -23,6 +23,8 @@ class IssueWithMetrics: """ + # pylint: disable=too-many-instance-attributes + def __init__( self, title, From 6c85d101ed728d9049c3eb3eb2f1eb1cf8dee6be Mon Sep 17 00:00:00 2001 From: Zack Koppert Date: Fri, 5 Apr 2024 16:45:40 -0700 Subject: [PATCH 11/18] Update test_most_active_mentors.py Co-authored-by: Jason Meridth --- test_most_active_mentors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_most_active_mentors.py b/test_most_active_mentors.py index 15edb95..9fc5db5 100755 --- a/test_most_active_mentors.py +++ b/test_most_active_mentors.py @@ -58,7 +58,7 @@ def test_get_mentor_count(self): """Test that get_mentor_count correctly counts comments per user.""" mentor_activity = {"sue": 15, "bob": 10} - # Create moc data + # Create mock data issues_with_metrics = [ IssueWithMetrics( "Issue 1", From 25063393ab3f757c1d5909a1bb676bb2231c253f Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Tue, 9 Apr 2024 09:23:11 +0200 Subject: [PATCH 12/18] Update config.py Co-authored-by: Jason Meridth --- config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.py b/config.py index df4f06a..4fc584f 100644 --- a/config.py +++ b/config.py @@ -183,7 +183,7 @@ def get_env_vars(test: bool = False) -> EnvVars: hide_time_to_answer = get_bool_env_var("HIDE_TIME_TO_ANSWER") hide_time_to_close = get_bool_env_var("HIDE_TIME_TO_CLOSE") hide_time_to_first_response = get_bool_env_var("HIDE_TIME_TO_FIRST_RESPONSE") - enable_mentor_count = os.getenv("ENABLE_MENTOR_COUNT", "FALSE") + enable_mentor_count = get_bool_env_var("ENABLE_MENTOR_COUNT") min_mentor_comments = os.getenv("MIN_MENTOR_COMMENTS", "10") max_comments_eval = os.getenv("MAX_COMMENTS_EVAL", "20") heavily_involved_cutoff = os.getenv("HEAVILY_INVOLVED_CUTOFF", "3") From c7c8509cdc2d5ebd7d34ffd569c774b13ecaf0d4 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Tue, 9 Apr 2024 09:24:19 +0200 Subject: [PATCH 13/18] Update config.py Remove merge residual --- config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/config.py b/config.py index 4fc584f..a6b7bbd 100644 --- a/config.py +++ b/config.py @@ -64,7 +64,6 @@ def __init__( self.gh_app_id = gh_app_id self.gh_app_installation_id = gh_app_installation_id self.gh_app_private_key_bytes = gh_app_private_key_bytes - self.search_query = search_query self.gh_token = gh_token self.ghe = ghe self.ignore_users = ignore_user From 3bfeec4b7e957d9f7faf5f633e3ac85e72ff9395 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Tue, 9 Apr 2024 09:25:54 +0200 Subject: [PATCH 14/18] Update requirements.txt Remove lib only needed for testing. --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 8e9f6e0..c1d2d5f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,3 @@ -flake8==7.0.0 github3.py==4.0.1 numpy==1.26.4 python-dotenv==1.0.1 From c5944927cd38fa14100d9dd3f181f8e72b110132 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Tue, 9 Apr 2024 09:26:50 +0200 Subject: [PATCH 15/18] Update issue_metrics.py Co-authored-by: Jason Meridth --- issue_metrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/issue_metrics.py b/issue_metrics.py index 8cd9834..f5c750a 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -334,7 +334,7 @@ def main(): stats_time_to_answer = get_stats_time_to_answer(issues_with_metrics) num_mentor_count = 0 - if enable_mentor_count == "TRUE": + if enable_mentor_count: num_mentor_count = get_mentor_count(issues_with_metrics, min_mentor_count) # Get stats describing the time in label for each label and store it in a dictionary From a0dca05959dce489c3453bc62702a7c011cbc9de Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 10 Apr 2024 10:47:33 +0200 Subject: [PATCH 16/18] Update config.py --- config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.py b/config.py index a6b7bbd..c62a904 100644 --- a/config.py +++ b/config.py @@ -34,7 +34,7 @@ class EnvVars: hide_time_to_first_response (bool): If true, the time to first response metric is hidden in the output ignore_users (List[str]): List of usernames to ignore when calculating metrics labels_to_measure (List[str]): List of labels to measure how much time the lable is applied - enable_mentor_count (str): If set to TRUE, compute number of mentors + enable_mentor_count (bool): If set to TRUE, compute number of mentors min_mentor_comments (str): If set, defines the minimum number of comments for mentors max_comments_eval (str): If set, defines the maximum number of comments to look at for mentor evaluation heavily_involved_cutoff (str): If set, defines the cutoff after which heavily involved commentors in From 4ba9d3c1904965fff6334c48293b196706deba00 Mon Sep 17 00:00:00 2001 From: Jason Meridth Date: Wed, 10 Apr 2024 23:58:13 -0500 Subject: [PATCH 17/18] Update config.py set type of `enable_mentor_count` to `bool` --- config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.py b/config.py index c62a904..30e91aa 100644 --- a/config.py +++ b/config.py @@ -55,7 +55,7 @@ def __init__( hide_time_to_first_response: bool, ignore_user: List[str], labels_to_measure: List[str], - enable_mentor_count: str, + enable_mentor_count: bool, min_mentor_comments: str, max_comments_eval: str, heavily_involved_cutoff: str, From 3f770f20c143bfa8593c70be85ba9c61df0197c7 Mon Sep 17 00:00:00 2001 From: Jason Meridth Date: Wed, 10 Apr 2024 23:59:19 -0500 Subject: [PATCH 18/18] Update test_config.py change tests to handle boolean change of enable_mentor_count --- test_config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test_config.py b/test_config.py index ef5fba7..d2f1bd1 100644 --- a/test_config.py +++ b/test_config.py @@ -118,7 +118,7 @@ def test_get_env_vars_with_github_app(self): False, [], [], - "FALSE", + False, "10", "20", "3", @@ -161,7 +161,7 @@ def test_get_env_vars_with_token(self): False, [], [], - "FALSE", + False, "10", "20", "3", @@ -203,7 +203,7 @@ def test_get_env_vars_optional_values(self): True, [], ["waiting-for-review", "waiting-for-manager"], - "FALSE", + False, 10, 20, 3,