From 5407da60aa590c81c9ce319a685d9a7dbfe33d43 Mon Sep 17 00:00:00 2001 From: Thomas Carmet <8408330+tcarmet@users.noreply.github.com> Date: Wed, 6 Mar 2024 21:12:58 +0000 Subject: [PATCH] Lint and rework comments on code --- bert_e/exceptions.py | 1 + bert_e/workflow/gitwaterflow/branches.py | 7 ------- bert_e/workflow/gitwaterflow/queueing.py | 10 +++++++++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/bert_e/exceptions.py b/bert_e/exceptions.py index 73552933..a349939b 100644 --- a/bert_e/exceptions.py +++ b/bert_e/exceptions.py @@ -238,6 +238,7 @@ class QueueBuildFailedMessage(TemplateException): code = 136 template = "queue_build_failed.md" + # internal exceptions class UnableToSendEmail(InternalException): code = 201 diff --git a/bert_e/workflow/gitwaterflow/branches.py b/bert_e/workflow/gitwaterflow/branches.py index f5c00688..238ec7ec 100644 --- a/bert_e/workflow/gitwaterflow/branches.py +++ b/bert_e/workflow/gitwaterflow/branches.py @@ -551,13 +551,6 @@ def validate(self): @property def failed_prs(self): """Return a list PRs in which the build have failed in the queue.""" - # TODO: Review if we should include the list of branches in the returned - # list of failed PRs. (To be used in the template to notify the user). - # Currently the drawback is that if the template changes a lot - # (one branch mentioned then two, then back to one) - # we will be sending multiple notifications to the user, with no good reason sometimes. - # But at the same time if we don't send them in some cases, the user will not be aware of - # the issue. if not self._validated: raise errors.QueuesNotValidated() diff --git a/bert_e/workflow/gitwaterflow/queueing.py b/bert_e/workflow/gitwaterflow/queueing.py index 41cecbc3..fa13f328 100644 --- a/bert_e/workflow/gitwaterflow/queueing.py +++ b/bert_e/workflow/gitwaterflow/queueing.py @@ -18,7 +18,7 @@ from bert_e import exceptions from bert_e.job import handler as job_handler -from bert_e.job import QueuesJob, PullRequestJob, Job +from bert_e.job import QueuesJob, PullRequestJob from bert_e.lib import git from ..git_utils import clone_git_repo, consecutive_merge, robust_merge, push @@ -36,6 +36,14 @@ def notify_queue_build_failed(failed_prs: List[int], job: QueuesJob): """Notify on the pull request that the queue build failed.""" + # TODO: As this feature evolves, we might want to include + # the list of failed q/ branches in the message. + # Currently the drawback is that if the template changes a lot + # (one branch mentioned then two, then back to one) + # we will be sending multiple notifications to the user, + # in some cases with no good reason, and in other cases with a good reason. + # This becomes less of an issue if we focus on notifying the user + # only through build status checks. for pr_id in failed_prs: pull_request = job.project_repo.get_pull_request(pr_id) send_comment(