From d2a0fb80305f40e1aef6f66b734bd1cd9704e73e Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Mon, 25 May 2026 17:17:54 +0200 Subject: [PATCH 1/7] Publish a comment to the Github PR mentioning the build errors --- bot/code_review_bot/report/__init__.py | 2 +- .../{mail_builderrors.py => builderrors.py} | 46 +++++++++++++------ 2 files changed, 33 insertions(+), 15 deletions(-) rename bot/code_review_bot/report/{mail_builderrors.py => builderrors.py} (70%) diff --git a/bot/code_review_bot/report/__init__.py b/bot/code_review_bot/report/__init__.py index ed79bb1ec..a0158fa34 100644 --- a/bot/code_review_bot/report/__init__.py +++ b/bot/code_review_bot/report/__init__.py @@ -4,10 +4,10 @@ import structlog +from code_review_bot.report.builderrors import BuildErrorsReporter from code_review_bot.report.github import GithubReporter from code_review_bot.report.lando import LandoReporter from code_review_bot.report.mail import MailReporter -from code_review_bot.report.mail_builderrors import BuildErrorsReporter from code_review_bot.report.phabricator import PhabricatorReporter logger = structlog.get_logger(__name__) diff --git a/bot/code_review_bot/report/mail_builderrors.py b/bot/code_review_bot/report/builderrors.py similarity index 70% rename from bot/code_review_bot/report/mail_builderrors.py rename to bot/code_review_bot/report/builderrors.py index fd57ace18..73698f33e 100644 --- a/bot/code_review_bot/report/mail_builderrors.py +++ b/bot/code_review_bot/report/builderrors.py @@ -6,7 +6,7 @@ from code_review_bot import taskcluster from code_review_bot.report.base import Reporter -from code_review_bot.revisions import PhabricatorRevision +from code_review_bot.revisions import GithubRevision, PhabricatorRevision logger = structlog.get_logger(__name__) @@ -19,10 +19,15 @@ {content}""" +# https://github.com/dead-claudia/github-limits#issue-comments +GITHUB_COMMENT_LIMIT = 65536 + class BuildErrorsReporter(Reporter): """ - Send an email to the author of the revision in case there are build errors + In case there are build errors, notify the author of the revision: + * By email in case of a Phabricator revision + * In a PR thread in case of a Github revision """ def __init__(self, configuration): @@ -32,15 +37,34 @@ def __init__(self, configuration): logger.info("BuildErrorsReporter report enabled.") def publish(self, issues, revision, task_failures, links, reviewers): - """ - Send an email to the author of the revision - """ - if not isinstance(revision, PhabricatorRevision): - logger.info( - "Skipping build error reporting, only available for Phabricator revisions" + build_errors = [issue for issue in issues if issue.is_build_error()] + + if not build_errors: + logger.info("No build errors encountered.") + return + + if isinstance(revision, GithubRevision): + # Comment directly to the pull request + + messages = [f"Hello @{revision.pull_request.user.login},"] + messages.append( + f"[Code Review bot](https://github.com/mozilla/code-review) detected {len(build_errors)} build errors when analyzing this Pull Request:" ) + for issue in build_errors: + messages.append(issue.as_error()) + + content = "\n".join(messages) + if len(content) > GITHUB_COMMENT_LIMIT: + content = content[: GITHUB_COMMENT_LIMIT - 1] + "…" + + revision.pull_request.create_issue_comment(content) return + elif not isinstance(revision, PhabricatorRevision): + raise NotImplementedError( + "Only Github and Phabricator revisions are supported" + ) + assert ( revision.phabricator_id and revision.phabricator_phid ), "PhabricatorRevision must have a Phabricator ID and PHID" @@ -56,12 +80,6 @@ def publish(self, issues, revision, task_failures, links, reviewers): ) return - build_errors = [issue for issue in issues if issue.is_build_error()] - - if not build_errors: - logger.info("No build errors encountered.") - return - content = EMAIL_HEADER.format( build_errors=len(build_errors), phabricator_id=revision.phabricator_id, From 8113c1cb888881b95ff86be01446d038acb5fe44 Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Mon, 25 May 2026 17:51:57 +0200 Subject: [PATCH 2/7] Update unit tests --- bot/tests/conftest.py | 1 + bot/tests/test_reporter_builderrors.py | 99 ++++++++++++++++++++++++++ bot/tests/test_reporter_mail.py | 46 ------------ 3 files changed, 100 insertions(+), 46 deletions(-) create mode 100644 bot/tests/test_reporter_builderrors.py diff --git a/bot/tests/conftest.py b/bot/tests/conftest.py index 57a7d5c1d..f4fa520f8 100644 --- a/bot/tests/conftest.py +++ b/bot/tests/conftest.py @@ -336,6 +336,7 @@ def mock_github(mock_config): json={ "url": "https://api.github.com/repos/owner/repo-name/pulls/1", "issue_url": "https://api.github.com/repos/owner/repo-name/pulls/1", + "user": {"login": "test_user"}, }, ) responses.add( diff --git a/bot/tests/test_reporter_builderrors.py b/bot/tests/test_reporter_builderrors.py new file mode 100644 index 000000000..123c90b5f --- /dev/null +++ b/bot/tests/test_reporter_builderrors.py @@ -0,0 +1,99 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import json +from pathlib import Path +from textwrap import dedent + +import responses +from conftest import FIXTURES_DIR +from responses import matchers + +from code_review_bot.report.builderrors import BuildErrorsReporter + +MAIL_CONTENT_BUILD_ERRORS = """ +# [Code Review bot](https://github.com/mozilla/code-review) found 2 build errors on [D51](https://phabricator.test/D51) + + +**Message**: ```Unidentified symbol``` +**Location**: some/file/path:0 + + +**Message**: ```Unidentified symbol``` +**Location**: some/file/path:1 +""" + + +def test_builderrors_taskcluster( + log, mock_config, mock_clang_tidy_issues, mock_revision, mock_taskcluster_config +): + """ + Test builderrors sending email to the author of the Phabricator revision + """ + + def _check_email(request): + payload = json.loads(request.body) + + assert payload["subject"] == "Code Review bot found 2 build errors on D51" + assert payload["address"] == "test@mozilla.com" + assert payload["content"] == MAIL_CONTENT_BUILD_ERRORS + + return (200, {}, "") # ack + + # Add mock taskcluster email to check output + responses.add_callback( + responses.POST, + "http://taskcluster.test/api/notify/v1/email", + callback=_check_email, + ) + + # Publish email + conf = {"emails": ["test@mozilla.com"]} + r = BuildErrorsReporter(conf) + + r.publish(mock_clang_tidy_issues, mock_revision, [], [], []) + + assert log.has("Send build error email", to="test@mozilla.com") + + +def test_builderrors_github( + log, + mock_config, + mock_clang_tidy_issues, + mock_github_revision, + mock_taskcluster_config, +): + """ + Test builderrors commenting on the github Pull Request to mention build errors + """ + mock_taskcluster_config.secrets = { + "REPORTERS": [ + { + "reporter": "github", + "client_id": "client_id", + "private_key_pem": (Path(FIXTURES_DIR) / "private_key.pem").read_text(), + "installation_id": 123456789, + } + ] + } + + responses.add( + responses.POST, + "https://api.github.com:443/repos/owner/repo-name/pulls/1/comments", + match=[ + matchers.json_params_matcher( + { + "body": dedent(""" + Hello @test_user, + [Code Review bot](https://github.com/mozilla/code-review) detected 1 build errors when analyzing this Pull Request: + + **Message**: ```Some Error Message``` + **Location**: dom/animation/Animation.cpp:57 + """).lstrip() + } + ) + ], + ) + r = BuildErrorsReporter({}) + r.publish(mock_clang_tidy_issues, mock_github_revision, [], [], []) diff --git a/bot/tests/test_reporter_mail.py b/bot/tests/test_reporter_mail.py index 650ad8416..58e1af4de 100644 --- a/bot/tests/test_reporter_mail.py +++ b/bot/tests/test_reporter_mail.py @@ -34,19 +34,6 @@ """ -MAIL_CONTENT_BUILD_ERRORS = """ -# [Code Review bot](https://github.com/mozilla/code-review) found 2 build errors on [D51](https://phabricator.test/D51) - - -**Message**: ```Unidentified symbol``` -**Location**: some/file/path:0 - - -**Message**: ```Unidentified symbol``` -**Location**: some/file/path:1 -""" - - def test_conf(mock_config, mock_taskcluster_config): """ Test mail reporter configuration @@ -137,36 +124,3 @@ def _check_email(request): "nb_errors": 2, } ] - - -def test_mail_builderrors( - log, mock_config, mock_clang_tidy_issues, mock_revision, mock_taskcluster_config -): - """ - Test mail_builderrors sending through Taskcluster - """ - from code_review_bot.report.mail_builderrors import BuildErrorsReporter - - def _check_email(request): - payload = json.loads(request.body) - - assert payload["subject"] == "Code Review bot found 2 build errors on D51" - assert payload["address"] == "test@mozilla.com" - assert payload["content"] == MAIL_CONTENT_BUILD_ERRORS - - return (200, {}, "") # ack - - # Add mock taskcluster email to check output - responses.add_callback( - responses.POST, - "http://taskcluster.test/api/notify/v1/email", - callback=_check_email, - ) - - # Publish email - conf = {"emails": ["test@mozilla.com"]} - r = BuildErrorsReporter(conf) - - r.publish(mock_clang_tidy_issues, mock_revision, [], [], []) - - assert log.has("Send build error email", to="test@mozilla.com") From d23845c1d608a3bc725fe1686269026d5d8ea522 Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Tue, 26 May 2026 11:11:09 +0200 Subject: [PATCH 3/7] Rebase on top of github-never-approves --- bot/code_review_bot/report/builderrors.py | 2 +- bot/code_review_bot/revisions/github.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/bot/code_review_bot/report/builderrors.py b/bot/code_review_bot/report/builderrors.py index 73698f33e..61aeaad57 100644 --- a/bot/code_review_bot/report/builderrors.py +++ b/bot/code_review_bot/report/builderrors.py @@ -57,7 +57,7 @@ def publish(self, issues, revision, task_failures, links, reviewers): if len(content) > GITHUB_COMMENT_LIMIT: content = content[: GITHUB_COMMENT_LIMIT - 1] + "…" - revision.pull_request.create_issue_comment(content) + revision.github_client.publish_comment(revision, content) return elif not isinstance(revision, PhabricatorRevision): diff --git a/bot/code_review_bot/revisions/github.py b/bot/code_review_bot/revisions/github.py index 21e4bb3de..0c96cfddf 100644 --- a/bot/code_review_bot/revisions/github.py +++ b/bot/code_review_bot/revisions/github.py @@ -90,7 +90,7 @@ def as_dict(self): } @cached_property - def pull_request(self): + def github_client(self): from code_review_bot.sources.github import GithubClient reporter_conf = next( @@ -103,12 +103,15 @@ def pull_request(self): ) # A github reporter configuration is required to perform a github Pull Request analysis assert reporter_conf, "Github reporter secrets must be set to access information about the pull request" - client = GithubClient( + return GithubClient( client_id=reporter_conf["client_id"], private_key=reporter_conf["private_key_pem"], installation_id=reporter_conf["installation_id"], ) - return client.get_pull_request(self) + + @cached_property + def pull_request(self): + return self.github_client.get_pull_request(self) def serialize(self): """ From 870f13d9b4b9ff78570d2528cf462999bc5a6b83 Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Tue, 26 May 2026 15:19:58 +0200 Subject: [PATCH 4/7] Move comment size limit to code_review_bot.sources.github --- bot/code_review_bot/report/builderrors.py | 6 ------ bot/code_review_bot/sources/github.py | 15 ++++++++++++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/bot/code_review_bot/report/builderrors.py b/bot/code_review_bot/report/builderrors.py index 61aeaad57..310907ffa 100644 --- a/bot/code_review_bot/report/builderrors.py +++ b/bot/code_review_bot/report/builderrors.py @@ -19,9 +19,6 @@ {content}""" -# https://github.com/dead-claudia/github-limits#issue-comments -GITHUB_COMMENT_LIMIT = 65536 - class BuildErrorsReporter(Reporter): """ @@ -54,9 +51,6 @@ def publish(self, issues, revision, task_failures, links, reviewers): messages.append(issue.as_error()) content = "\n".join(messages) - if len(content) > GITHUB_COMMENT_LIMIT: - content = content[: GITHUB_COMMENT_LIMIT - 1] + "…" - revision.github_client.publish_comment(revision, content) return diff --git a/bot/code_review_bot/sources/github.py b/bot/code_review_bot/sources/github.py index 7c3244aaa..638f68dfd 100644 --- a/bot/code_review_bot/sources/github.py +++ b/bot/code_review_bot/sources/github.py @@ -17,6 +17,11 @@ logger = structlog.get_logger(__name__) +# Github does not provide an exact limit for the body content on its API. +# We prefer using a safe interval here, based on https://github.com/dead-claudia/github-limits#issue-comments +GITHUB_COMMENT_LIMIT = 65536 + + class ReviewEvent(enum.Enum): """ Review action you want to perform. @@ -56,10 +61,14 @@ def get_pull_request(self, revision: GithubRevision): return repo.get_pull(revision.pull_number) def _build_review_comment(self, issue): + message = issue.message + if len(message) > GITHUB_COMMENT_LIMIT: + message = message[: GITHUB_COMMENT_LIMIT - 1] + "…" + return ReviewComment( path=issue.path, line=issue.line, - body=issue.message, + body=message, ) def publish_comment( @@ -73,6 +82,10 @@ def publish_comment( assert isinstance(revision, GithubRevision), "Only for github revisions" pull_request = self.get_pull_request(revision) + + if len(message) > GITHUB_COMMENT_LIMIT: + message = message[: GITHUB_COMMENT_LIMIT - 1] + "…" + pull_request.create_issue_comment(body=message) def publish_review( From 3a8abdb234f35af32579bf4e7ed43984f8e0c46f Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Tue, 26 May 2026 15:49:43 +0200 Subject: [PATCH 5/7] Use a class method to initialize Github client from the configuration --- bot/code_review_bot/report/builderrors.py | 12 +++++++++--- bot/code_review_bot/report/github.py | 12 ++++++------ bot/code_review_bot/revisions/github.py | 17 +++++------------ bot/code_review_bot/sources/github.py | 17 +++++++++++++++++ 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/bot/code_review_bot/report/builderrors.py b/bot/code_review_bot/report/builderrors.py index 310907ffa..6a464923c 100644 --- a/bot/code_review_bot/report/builderrors.py +++ b/bot/code_review_bot/report/builderrors.py @@ -7,6 +7,7 @@ from code_review_bot import taskcluster from code_review_bot.report.base import Reporter from code_review_bot.revisions import GithubRevision, PhabricatorRevision +from code_review_bot.sources.github import GithubClient logger = structlog.get_logger(__name__) @@ -30,7 +31,7 @@ class BuildErrorsReporter(Reporter): def __init__(self, configuration): # Load TC services self.notify = taskcluster.get_service("notify") - + self.github_client = GithubClient.from_configuration(configuration) logger.info("BuildErrorsReporter report enabled.") def publish(self, issues, revision, task_failures, links, reviewers): @@ -41,8 +42,13 @@ def publish(self, issues, revision, task_failures, links, reviewers): return if isinstance(revision, GithubRevision): - # Comment directly to the pull request + if not self.github_client: + logger.error( + "Github API client is not initialized, skipping Github reporting" + ) + return + # Comment directly to the pull request messages = [f"Hello @{revision.pull_request.user.login},"] messages.append( f"[Code Review bot](https://github.com/mozilla/code-review) detected {len(build_errors)} build errors when analyzing this Pull Request:" @@ -51,7 +57,7 @@ def publish(self, issues, revision, task_failures, links, reviewers): messages.append(issue.as_error()) content = "\n".join(messages) - revision.github_client.publish_comment(revision, content) + self.github_client.publish_comment(revision, content) return elif not isinstance(revision, PhabricatorRevision): diff --git a/bot/code_review_bot/report/github.py b/bot/code_review_bot/report/github.py index 9a3498b4b..e81031f3e 100644 --- a/bot/code_review_bot/report/github.py +++ b/bot/code_review_bot/report/github.py @@ -19,12 +19,7 @@ def __init__(self, configuration={}, *args, **kwargs): if not configuration.get(key): raise Exception(f"Missing github reporter configuration key {key}") - # Setup github App secret from the configuration - self.github_client = GithubClient( - client_id=configuration["client_id"], - private_key=configuration["private_key_pem"], - installation_id=configuration["installation_id"], - ) + self.github_client = GithubClient.from_configuration(configuration) self.analyzers_skipped = configuration.get("analyzers_skipped", []) assert isinstance( @@ -40,6 +35,11 @@ def publish(self, issues, revision, task_failures, notices, reviewers): "Skipping github reporting, only available for Github revisions" ) return + if not self.github_client: + logger.error( + "Github API client is not initialized, skipping Github reporting" + ) + return if reviewers: logger.warn( diff --git a/bot/code_review_bot/revisions/github.py b/bot/code_review_bot/revisions/github.py index 0c96cfddf..249b34758 100644 --- a/bot/code_review_bot/revisions/github.py +++ b/bot/code_review_bot/revisions/github.py @@ -90,9 +90,10 @@ def as_dict(self): } @cached_property - def github_client(self): + def pull_request(self): from code_review_bot.sources.github import GithubClient + # Setup Github API client, needed to get PR information reporter_conf = next( ( reporter @@ -101,17 +102,9 @@ def github_client(self): ), None, ) - # A github reporter configuration is required to perform a github Pull Request analysis - assert reporter_conf, "Github reporter secrets must be set to access information about the pull request" - return GithubClient( - client_id=reporter_conf["client_id"], - private_key=reporter_conf["private_key_pem"], - installation_id=reporter_conf["installation_id"], - ) - - @cached_property - def pull_request(self): - return self.github_client.get_pull_request(self) + assert reporter_conf, "Github reporter secrets must be set to access information about the Pull Request" + github_client = GithubClient.from_configuration(reporter_conf) + return github_client.get_pull_request(self) def serialize(self): """ diff --git a/bot/code_review_bot/sources/github.py b/bot/code_review_bot/sources/github.py index 638f68dfd..d85461fb7 100644 --- a/bot/code_review_bot/sources/github.py +++ b/bot/code_review_bot/sources/github.py @@ -55,6 +55,23 @@ def __init__(self, client_id: str, private_key: str, installation_id: str): self.review_comments = [] + @classmethod + def from_configuration(cls, configuration: dict): + """Setup github App secrets from the configuration""" + if not all( + configuration.get(key) + for key in ("client_id", "private_key_pem", "installation_id") + ): + logger.warning( + "Missing github reporter configuration key. Github API client was not initialized" + ) + return + return cls( + client_id=configuration["client_id"], + private_key=configuration["private_key_pem"], + installation_id=configuration["installation_id"], + ) + @lru_cache def get_pull_request(self, revision: GithubRevision): repo = self.api.get_repo(revision.repo_name) From 3b3771c1454031f7a4616349b906d5895d37b25d Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Tue, 26 May 2026 15:58:22 +0200 Subject: [PATCH 6/7] Use separate functions to publish build errors --- bot/code_review_bot/report/builderrors.py | 117 ++++++++++++---------- 1 file changed, 65 insertions(+), 52 deletions(-) diff --git a/bot/code_review_bot/report/builderrors.py b/bot/code_review_bot/report/builderrors.py index 6a464923c..32418c838 100644 --- a/bot/code_review_bot/report/builderrors.py +++ b/bot/code_review_bot/report/builderrors.py @@ -4,7 +4,7 @@ import structlog -from code_review_bot import taskcluster +from code_review_bot import Issue, taskcluster from code_review_bot.report.base import Reporter from code_review_bot.revisions import GithubRevision, PhabricatorRevision from code_review_bot.sources.github import GithubClient @@ -34,21 +34,16 @@ def __init__(self, configuration): self.github_client = GithubClient.from_configuration(configuration) logger.info("BuildErrorsReporter report enabled.") - def publish(self, issues, revision, task_failures, links, reviewers): - build_errors = [issue for issue in issues if issue.is_build_error()] - - if not build_errors: - logger.info("No build errors encountered.") - return - - if isinstance(revision, GithubRevision): + def publish_github(self, revision: GithubRevision, build_errors: list[Issue]): + """ + Comment directly on the Pull Request, as a comment mentioning the author + """ if not self.github_client: logger.error( "Github API client is not initialized, skipping Github reporting" ) return - # Comment directly to the pull request messages = [f"Hello @{revision.pull_request.user.login},"] messages.append( f"[Code Review bot](https://github.com/mozilla/code-review) detected {len(build_errors)} build errors when analyzing this Pull Request:" @@ -58,56 +53,74 @@ def publish(self, issues, revision, task_failures, links, reviewers): content = "\n".join(messages) self.github_client.publish_comment(revision, content) - return - elif not isinstance(revision, PhabricatorRevision): - raise NotImplementedError( - "Only Github and Phabricator revisions are supported" + def publish_phabricator( + self, revision: PhabricatorRevision, build_errors: list[Issue] + ): + """ + Notify by email the author of the last commit for this revision + """ + assert ( + revision.phabricator_id and revision.phabricator_phid + ), "PhabricatorRevision must have a Phabricator ID and PHID" + assert ( + "attachments" in revision.diff + ), f"Unable to find the commits for revision with phid {revision.phabricator_phid}." + + attachments = revision.diff["attachments"] + + if "commits" not in attachments and "commits" not in attachments["commits"]: + logger.info( + f"Unable to find the commits for revision with phid {revision.phabricator_phid}." + ) + return + + content = EMAIL_HEADER.format( + build_errors=len(build_errors), + phabricator_id=revision.phabricator_id, + review_url=revision.url, + content="\n".join([i.as_error() for i in build_errors]), ) - assert ( - revision.phabricator_id and revision.phabricator_phid - ), "PhabricatorRevision must have a Phabricator ID and PHID" - assert ( - "attachments" in revision.diff - ), f"Unable to find the commits for revision with phid {revision.phabricator_phid}." + if len(content) > 102400: + # Content is 102400 chars max + content = content[:102000] + "\n\n... Content max limit reached!" - attachments = revision.diff["attachments"] + # Get the last commit + commit = attachments["commits"]["commits"][-1] - if "commits" not in attachments and "commits" not in attachments["commits"]: - logger.info( - f"Unable to find the commits for revision with phid {revision.phabricator_phid}." + if "author" not in commit: + logger.info("Unable to find the author for commit.") + return + + logger.info("Send build error email", to=commit["author"]["email"]) + + # Since we now know that there is an "author" field we assume that we have "email" + self.notify.email( + { + "address": commit["author"]["email"], + "subject": EMAIL_SUBJECT.format( + build_errors=len(build_errors), + phabricator_id=revision.phabricator_id, + ), + "content": content, + } ) - return - content = EMAIL_HEADER.format( - build_errors=len(build_errors), - phabricator_id=revision.phabricator_id, - review_url=revision.url, - content="\n".join([i.as_error() for i in build_errors]), - ) + def publish(self, issues, revision, task_failures, links, reviewers): + build_errors = [issue for issue in issues if issue.is_build_error()] - if len(content) > 102400: - # Content is 102400 chars max - content = content[:102000] + "\n\n... Content max limit reached!" + if not build_errors: + logger.info("No build errors encountered.") + return - # Get the last commit - commit = attachments["commits"]["commits"][-1] + if isinstance(revision, GithubRevision): + self.publish_github(revision, build_errors) - if "author" not in commit: - logger.info("Unable to find the author for commit.") - return + elif isinstance(revision, PhabricatorRevision): + self.publish_phabricator(revision, build_errors) - logger.info("Send build error email", to=commit["author"]["email"]) - - # Since we nw know that there is an "author" field we assume that we have "email" - self.notify.email( - { - "address": commit["author"]["email"], - "subject": EMAIL_SUBJECT.format( - build_errors=len(build_errors), - phabricator_id=revision.phabricator_id, - ), - "content": content, - } - ) + else: + raise NotImplementedError( + "Only Github and Phabricator revisions are supported" + ) From 66bcf7ef2906f194a6482384218a34ddb53a6912 Mon Sep 17 00:00:00 2001 From: Valentin Rigal Date: Tue, 26 May 2026 18:41:08 +0200 Subject: [PATCH 7/7] Fix indent issue --- bot/code_review_bot/report/builderrors.py | 138 +++++++++++----------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/bot/code_review_bot/report/builderrors.py b/bot/code_review_bot/report/builderrors.py index 32418c838..0dfbeaf83 100644 --- a/bot/code_review_bot/report/builderrors.py +++ b/bot/code_review_bot/report/builderrors.py @@ -34,78 +34,78 @@ def __init__(self, configuration): self.github_client = GithubClient.from_configuration(configuration) logger.info("BuildErrorsReporter report enabled.") - def publish_github(self, revision: GithubRevision, build_errors: list[Issue]): - """ - Comment directly on the Pull Request, as a comment mentioning the author - """ - if not self.github_client: - logger.error( - "Github API client is not initialized, skipping Github reporting" - ) - return - - messages = [f"Hello @{revision.pull_request.user.login},"] - messages.append( - f"[Code Review bot](https://github.com/mozilla/code-review) detected {len(build_errors)} build errors when analyzing this Pull Request:" - ) - for issue in build_errors: - messages.append(issue.as_error()) - - content = "\n".join(messages) - self.github_client.publish_comment(revision, content) - - def publish_phabricator( - self, revision: PhabricatorRevision, build_errors: list[Issue] - ): - """ - Notify by email the author of the last commit for this revision - """ - assert ( - revision.phabricator_id and revision.phabricator_phid - ), "PhabricatorRevision must have a Phabricator ID and PHID" - assert ( - "attachments" in revision.diff - ), f"Unable to find the commits for revision with phid {revision.phabricator_phid}." - - attachments = revision.diff["attachments"] - - if "commits" not in attachments and "commits" not in attachments["commits"]: - logger.info( - f"Unable to find the commits for revision with phid {revision.phabricator_phid}." - ) - return - - content = EMAIL_HEADER.format( - build_errors=len(build_errors), - phabricator_id=revision.phabricator_id, - review_url=revision.url, - content="\n".join([i.as_error() for i in build_errors]), + def publish_github(self, revision: GithubRevision, build_errors: list[Issue]): + """ + Comment directly on the Pull Request, as a comment mentioning the author + """ + if not self.github_client: + logger.error( + "Github API client is not initialized, skipping Github reporting" ) + return - if len(content) > 102400: - # Content is 102400 chars max - content = content[:102000] + "\n\n... Content max limit reached!" - - # Get the last commit - commit = attachments["commits"]["commits"][-1] - - if "author" not in commit: - logger.info("Unable to find the author for commit.") - return - - logger.info("Send build error email", to=commit["author"]["email"]) - - # Since we now know that there is an "author" field we assume that we have "email" - self.notify.email( - { - "address": commit["author"]["email"], - "subject": EMAIL_SUBJECT.format( - build_errors=len(build_errors), - phabricator_id=revision.phabricator_id, - ), - "content": content, - } + messages = [f"Hello @{revision.pull_request.user.login},"] + messages.append( + f"[Code Review bot](https://github.com/mozilla/code-review) detected {len(build_errors)} build errors when analyzing this Pull Request:" + ) + for issue in build_errors: + messages.append(issue.as_error()) + + content = "\n".join(messages) + self.github_client.publish_comment(revision, content) + + def publish_phabricator( + self, revision: PhabricatorRevision, build_errors: list[Issue] + ): + """ + Notify by email the author of the last commit for this revision + """ + assert ( + revision.phabricator_id and revision.phabricator_phid + ), "PhabricatorRevision must have a Phabricator ID and PHID" + assert ( + "attachments" in revision.diff + ), f"Unable to find the commits for revision with phid {revision.phabricator_phid}." + + attachments = revision.diff["attachments"] + + if "commits" not in attachments and "commits" not in attachments["commits"]: + logger.info( + f"Unable to find the commits for revision with phid {revision.phabricator_phid}." ) + return + + content = EMAIL_HEADER.format( + build_errors=len(build_errors), + phabricator_id=revision.phabricator_id, + review_url=revision.url, + content="\n".join([i.as_error() for i in build_errors]), + ) + + if len(content) > 102400: + # Content is 102400 chars max + content = content[:102000] + "\n\n... Content max limit reached!" + + # Get the last commit + commit = attachments["commits"]["commits"][-1] + + if "author" not in commit: + logger.info("Unable to find the author for commit.") + return + + logger.info("Send build error email", to=commit["author"]["email"]) + + # Since we now know that there is an "author" field we assume that we have "email" + self.notify.email( + { + "address": commit["author"]["email"], + "subject": EMAIL_SUBJECT.format( + build_errors=len(build_errors), + phabricator_id=revision.phabricator_id, + ), + "content": content, + } + ) def publish(self, issues, revision, task_failures, links, reviewers): build_errors = [issue for issue in issues if issue.is_build_error()]