From e846502b48d156d590a9bcf727cae38a04868e4e Mon Sep 17 00:00:00 2001 From: "Brian S. Stephan" Date: Fri, 24 Jun 2016 13:50:09 -0500 Subject: [PATCH] gitlab bot: start refactoring code review methods the code review logic to this point is kind of convoluted, and still is for the moment, but this moves some of the stuff into a more coherent spot in the expectation of reuse. most of the refactoring is still to come --- dr_botzo/gitlab_bot/lib.py | 115 +++++++++--------- .../management/commands/code_review_scan.py | 4 +- 2 files changed, 61 insertions(+), 58 deletions(-) diff --git a/dr_botzo/gitlab_bot/lib.py b/dr_botzo/gitlab_bot/lib.py index cc13e2c..5df85f1 100644 --- a/dr_botzo/gitlab_bot/lib.py +++ b/dr_botzo/gitlab_bot/lib.py @@ -91,68 +91,73 @@ class GitlabBot(object): return approve_count, approvers, send_reset - def scan_for_reviews(self, project, project_obj, merge_request): - log.debug("scanning merge request '%s'", merge_request.title) + def scan_project_for_reviews(self, project): + project_obj = self.client.projects.get(project.project_id) + if not project_obj: + return - # check to see if the merge request needs a reviewer or a merge - approve_count, approvers, send_reset = self.get_code_review_acceptance_count(merge_request) - if approve_count < project.code_reviews_necessary: - log.debug("%s needs a code review", merge_request.title) - if merge_request.assignee is not None: - log.debug("%s currently assigned to %s", merge_request.title, merge_request.assignee.username) - excluded_candidates = approvers + [merge_request.author.username] - candidates = [x for x in project.code_reviewers.split(',') if x not in excluded_candidates] + for merge_request in project_obj.mergerequests.list(state='opened'): + log.debug("scanning merge request '%s'", merge_request.title) - # if the current assignee is in candidates, then we don't need to do anything - if merge_request.assignee is None or merge_request.assignee.username not in candidates: - if len(candidates) > 0: - new_reviewer = random.choice(candidates) - log.debug("%s is the new reviewer", new_reviewer) + # check to see if the merge request needs a reviewer or a merge + approve_count, approvers, send_reset = self.get_code_review_acceptance_count(merge_request) + if approve_count < project.code_reviews_necessary: + log.debug("%s needs a code review", merge_request.title) + if merge_request.assignee is not None: + log.debug("%s currently assigned to %s", merge_request.title, merge_request.assignee.username) + excluded_candidates = approvers + [merge_request.author.username] + candidates = [x for x in project.code_reviewers.split(',') if x not in excluded_candidates] - # find the user object for the new reviewer - new_reviewer_obj = self.client.users.get_by_username(new_reviewer) + # if the current assignee is in candidates, then we don't need to do anything + if merge_request.assignee is None or merge_request.assignee.username not in candidates: + if len(candidates) > 0: + new_reviewer = random.choice(candidates) + log.debug("%s is the new reviewer", new_reviewer) - # create note for the update - assign_msg = {'body': self.random_new_reviewer_message().format(approve_count, - project.code_reviews_necessary, - new_reviewer)} - self.client.project_mergerequest_notes.create(assign_msg, project_id=project_obj.id, - merge_request_id=merge_request.id) + # find the user object for the new reviewer + new_reviewer_obj = self.client.users.get_by_username(new_reviewer) - # assign the merge request to the new reviewer - self.client.update(merge_request, assignee_id=new_reviewer_obj.id) + # create note for the update + assign_msg = {'body': self.random_new_reviewer_message().format(approve_count, + project.code_reviews_necessary, + new_reviewer)} + self.client.project_mergerequest_notes.create(assign_msg, project_id=project_obj.id, + merge_request_id=merge_request.id) + + # assign the merge request to the new reviewer + self.client.update(merge_request, assignee_id=new_reviewer_obj.id) + else: + log.warning("no reviewers left to review %s, doing nothing", merge_request.title) else: - log.warning("no reviewers left to review %s, doing nothing", merge_request.title) + log.info("%s is already assigned to a candidate reviewer", merge_request.title) + + # regardless of our decision to assign or not, if we're supposed to send the reset + # message, we should + if send_reset: + log.info("adding reset message to %s", merge_request.title) + reset_msg = {'body': self.random_counter_reset_message().format(project.code_reviews_necessary)} + self.client.project_mergerequest_notes.create(reset_msg, project_id=project_obj.id, + merge_request_id=merge_request.id) else: - log.info("%s is already assigned to a candidate reviewer", merge_request.title) + log.debug("%s has enough code reviews, it needs someone to merge it", merge_request.title) + if merge_request.assignee is not None: + log.debug("%s currently assigned to %s", merge_request.title, merge_request.assignee.username) + excluded = [merge_request.author.username] + candidates = [x for x in project.code_review_final_merge_assignees.split(',') if x not in excluded] + if merge_request.assignee is None or merge_request.assignee.username not in candidates: + if len(candidates) > 0: + new_approver = random.choice(candidates) + log.debug("%s is the new approver", new_approver) - # regardless of our decision to assign or not, if we're supposed to send the reset - # message, we should - if send_reset: - log.info("adding reset message to %s", merge_request.title) - reset_msg = {'body': self.random_counter_reset_message().format(project.code_reviews_necessary)} - self.client.project_mergerequest_notes.create(reset_msg, project_id=project_obj.id, - merge_request_id=merge_request.id) - else: - log.debug("%s has enough code reviews, it needs someone to merge it", merge_request.title) - if merge_request.assignee is not None: - log.debug("%s currently assigned to %s", merge_request.title, merge_request.assignee.username) - excluded = [merge_request.author.username] - candidates = [x for x in project.code_review_final_merge_assignees.split(',') if x not in excluded] - if merge_request.assignee is None or merge_request.assignee.username not in candidates: - if len(candidates) > 0: - new_approver = random.choice(candidates) - log.debug("%s is the new approver", new_approver) + # find the user object for the new reviewer + new_approver_obj = self.client.users.get_by_username(new_approver) - # find the user object for the new reviewer - new_approver_obj = self.client.users.get_by_username(new_approver) + # create note for the update + assign_msg = {'body': self.random_reviews_done_message().format(new_approver)} + self.client.project_mergerequest_notes.create(assign_msg, project_id=project_obj.id, + merge_request_id=merge_request.id) - # create note for the update - assign_msg = {'body': self.random_reviews_done_message().format(new_approver)} - self.client.project_mergerequest_notes.create(assign_msg, project_id=project_obj.id, - merge_request_id=merge_request.id) - - # assign the merge request to the new reviewer - self.client.update(merge_request, assignee_id=new_approver_obj.id) - else: - log.warning("no approvers left to approve %s, doing nothing", merge_request.title) + # assign the merge request to the new reviewer + self.client.update(merge_request, assignee_id=new_approver_obj.id) + else: + log.warning("no approvers left to approve %s, doing nothing", merge_request.title) diff --git a/dr_botzo/gitlab_bot/management/commands/code_review_scan.py b/dr_botzo/gitlab_bot/management/commands/code_review_scan.py index cb48931..b4a66bf 100644 --- a/dr_botzo/gitlab_bot/management/commands/code_review_scan.py +++ b/dr_botzo/gitlab_bot/management/commands/code_review_scan.py @@ -17,6 +17,4 @@ class Command(BaseCommand): bot = GitlabBot() projects = GitlabProjectConfig.objects.filter(manage_merge_request_code_reviews=True) for project in projects: - project_obj = bot.client.projects.get(project.project_id) - for merge_request in project_obj.mergerequests.list(state='opened'): - bot.scan_for_reviews(project, project_obj, merge_request) + bot.scan_project_for_reviews(project)