From 5730ba9a015ca053591e6e277ccc097cb4f02fee Mon Sep 17 00:00:00 2001 From: Maxwell G Date: Wed, 4 Oct 2023 18:28:05 +0000 Subject: [PATCH 1/6] pr_labeler: improve create_boilerplate_comment logging --- hacking/pr_labeler/label.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hacking/pr_labeler/label.py b/hacking/pr_labeler/label.py index 118b281a67f..3ed919cf1fe 100644 --- a/hacking/pr_labeler/label.py +++ b/hacking/pr_labeler/label.py @@ -166,7 +166,10 @@ def create_boilerplate_comment(ctx: IssueOrPrCtx, name: str, **kwargs) -> None: if comment.body.splitlines()[-1] == last: log(ctx, name, "boilerplate was already commented") return - log(ctx, "Templating", name, "boilerplate") + msg = f"Templating {name} boilerplate" + if kwargs: + msg += f" with {kwargs}" + log(ctx, msg) create_comment(ctx, tmpl) From 44ffe0f2105f6154234aa40c2a99c51597c6fd7d Mon Sep 17 00:00:00 2001 From: Maxwell G Date: Wed, 4 Oct 2023 18:28:33 +0000 Subject: [PATCH 2/6] pr_labeler: add --force-process-closed flag --- hacking/pr_labeler/label.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hacking/pr_labeler/label.py b/hacking/pr_labeler/label.py index 3ed919cf1fe..265d4b5e420 100644 --- a/hacking/pr_labeler/label.py +++ b/hacking/pr_labeler/label.py @@ -246,6 +246,7 @@ def process_pr( pr_number: int, dry_run: bool = False, authed_dry_run: bool = False, + force_process_closed: bool = False, ) -> None: global_args = click_ctx.ensure_object(GlobalArgs) @@ -266,7 +267,7 @@ def process_pr( event_info=get_event_info(), issue=pr.as_issue(), ) - if pr.state != "open": + if not force_process_closed and pr.state != "open": log(ctx, "Refusing to process closed ticket") return @@ -282,6 +283,7 @@ def process_issue( issue_number: int, dry_run: bool = False, authed_dry_run: bool = False, + force_process_closed: bool = False, ) -> None: global_args = click_ctx.ensure_object(GlobalArgs) @@ -300,7 +302,7 @@ def process_issue( dry_run=dry_run, event_info=get_event_info(), ) - if issue.state != "open": + if not force_process_closed and issue.state != "open": log(ctx, "Refusing to process closed ticket") return From d2e6625e8bb64bf831eb8b31582c654f5333901a Mon Sep 17 00:00:00 2001 From: Maxwell G Date: Wed, 4 Oct 2023 18:29:12 +0000 Subject: [PATCH 3/6] pr_labeler: add warning for porting_guides changes This adds a warning message when PRs are created that edit porting_guides by someone outside of the Release Management WG. These files are automatically generated during the ansible release process and should not be modified. Fixes: https://github.com/ansible/ansible-documentation/issues/503 --- .../pr_labeler/data/porting_guide_changes.md | 8 +++++ hacking/pr_labeler/label.py | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 hacking/pr_labeler/data/porting_guide_changes.md diff --git a/hacking/pr_labeler/data/porting_guide_changes.md b/hacking/pr_labeler/data/porting_guide_changes.md new file mode 100644 index 00000000000..0e0cc12e3fe --- /dev/null +++ b/hacking/pr_labeler/data/porting_guide_changes.md @@ -0,0 +1,8 @@ +The following files are automatically generated and should not be modified outside of the ansible release process: + +{% for file in changed_files %} +- {{ file }} +{% endfor %} + +Please double check your changes. + diff --git a/hacking/pr_labeler/label.py b/hacking/pr_labeler/label.py index 265d4b5e420..e13f13c47eb 100644 --- a/hacking/pr_labeler/label.py +++ b/hacking/pr_labeler/label.py @@ -6,6 +6,7 @@ import dataclasses import json import os +import re from collections.abc import Collection from contextlib import suppress from functools import cached_property @@ -26,6 +27,14 @@ LABELS_BY_CODEOWNER: dict[OwnerTuple, list[str]] = { ("TEAM", "@ansible/steering-committee"): ["sc_approval"], } +RELEASE_MANAGEMENT_MEMBERS = { + "anweshadas", + "felixfontein", + "gotmax23", + "mariolenz", + "rooftopcellist", + "webknjaz", +} HERE = Path(__file__).resolve().parent ROOT = HERE.parent.parent CODEOWNERS = (ROOT / ".github/CODEOWNERS").read_text("utf-8") @@ -228,6 +237,25 @@ def no_body_nag(ctx: IssueOrPrCtx) -> None: create_boilerplate_comment(ctx, "no_body_nag.md") +def warn_porting_guide_change(ctx: PRLabelerCtx) -> None: + """ + Complain if a user outside of the Release Management WG changes porting_guide + """ + if ctx.pr.user.login in RELEASE_MANAGEMENT_MEMBERS: + return + matches: list[str] = [] + for file in ctx.pr.get_files(): + if re.fullmatch( + # Match community porting guides but not core porting guides + r"docs/docsite/rst/porting_guides/porting_guide_\d.*.rst", + file.filename, + ): + matches.append(file.filename) + if not matches: + return + create_boilerplate_comment(ctx, "porting_guide_changes.md", changed_files=matches) + + APP = typer.Typer() @@ -274,6 +302,7 @@ def process_pr( handle_codeowner_labels(ctx) new_contributor_welcome(ctx) no_body_nag(ctx) + warn_porting_guide_change(ctx) @APP.command(name="issue") From dddfd7eb55fae94241a8da054ce8245293c8576f Mon Sep 17 00:00:00 2001 From: Maxwell G Date: Tue, 17 Oct 2023 21:54:17 +0000 Subject: [PATCH 4/6] pr_labeler: use @release-management-wg team for porting_guide check Instead of hardcoding the list of release managers, we can use the Github API to retrieve the members of the `@ansible/release-management-wg` team. --- hacking/pr_labeler/label.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/hacking/pr_labeler/label.py b/hacking/pr_labeler/label.py index e13f13c47eb..6fa0ed97e92 100644 --- a/hacking/pr_labeler/label.py +++ b/hacking/pr_labeler/label.py @@ -27,14 +27,6 @@ LABELS_BY_CODEOWNER: dict[OwnerTuple, list[str]] = { ("TEAM", "@ansible/steering-committee"): ["sc_approval"], } -RELEASE_MANAGEMENT_MEMBERS = { - "anweshadas", - "felixfontein", - "gotmax23", - "mariolenz", - "rooftopcellist", - "webknjaz", -} HERE = Path(__file__).resolve().parent ROOT = HERE.parent.parent CODEOWNERS = (ROOT / ".github/CODEOWNERS").read_text("utf-8") @@ -182,6 +174,18 @@ def create_boilerplate_comment(ctx: IssueOrPrCtx, name: str, **kwargs) -> None: create_comment(ctx, tmpl) +def get_team_members(ctx: IssueOrPrCtx, team: str) -> list[str]: + """ + Get the members of a Github team + """ + return [ + user.login + for user in ctx.client.get_organization(ctx.repo.organization.login) + .get_team_by_slug(team) + .get_members() + ] + + def handle_codeowner_labels(ctx: PRLabelerCtx) -> None: labels = LABELS_BY_CODEOWNER.copy() owners = CodeOwners(CODEOWNERS) @@ -241,8 +245,16 @@ def warn_porting_guide_change(ctx: PRLabelerCtx) -> None: """ Complain if a user outside of the Release Management WG changes porting_guide """ - if ctx.pr.user.login in RELEASE_MANAGEMENT_MEMBERS: + # If the API token does not have permisisons to view teams in the ansible + # org, fall back to an empty list. + members = [] + try: + members = get_team_members(ctx, "release-management-wg") + except github.UnknownObjectException: + log(ctx, "Failed to get members of @ansible/release-management-wg") + if ctx.pr.user.login in members: return + matches: list[str] = [] for file in ctx.pr.get_files(): if re.fullmatch( From 746662c25577a1ba0bbfcac69b2b0ced28d75343 Mon Sep 17 00:00:00 2001 From: Maxwell G Date: Wed, 18 Oct 2023 02:37:17 +0000 Subject: [PATCH 5/6] pr_labeler: exempt bots from porting_guide check For example, patchback is not a release manager, but we still want it to backport Porting Guide PRs. --- hacking/pr_labeler/label.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hacking/pr_labeler/label.py b/hacking/pr_labeler/label.py index 6fa0ed97e92..29c4a468071 100644 --- a/hacking/pr_labeler/label.py +++ b/hacking/pr_labeler/label.py @@ -243,8 +243,13 @@ def no_body_nag(ctx: IssueOrPrCtx) -> None: def warn_porting_guide_change(ctx: PRLabelerCtx) -> None: """ - Complain if a user outside of the Release Management WG changes porting_guide + Complain if a non-bot user outside of the Release Management WG changes + porting_guide """ + user = ctx.pr.user.login + if user.endswith("[bot]"): + return + # If the API token does not have permisisons to view teams in the ansible # org, fall back to an empty list. members = [] @@ -252,7 +257,7 @@ def warn_porting_guide_change(ctx: PRLabelerCtx) -> None: members = get_team_members(ctx, "release-management-wg") except github.UnknownObjectException: log(ctx, "Failed to get members of @ansible/release-management-wg") - if ctx.pr.user.login in members: + if user in members: return matches: list[str] = [] From 95ece7e9d6dc8e411dd729fc7c7ef48500c04d15 Mon Sep 17 00:00:00 2001 From: Maxwell G Date: Wed, 18 Oct 2023 02:45:08 +0000 Subject: [PATCH 6/6] pr_labeler: improve porting_guide_changes template wording Co-authored-by: Sandra McCann --- hacking/pr_labeler/data/porting_guide_changes.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hacking/pr_labeler/data/porting_guide_changes.md b/hacking/pr_labeler/data/porting_guide_changes.md index 0e0cc12e3fe..1bab844220d 100644 --- a/hacking/pr_labeler/data/porting_guide_changes.md +++ b/hacking/pr_labeler/data/porting_guide_changes.md @@ -1,8 +1,8 @@ -The following files are automatically generated and should not be modified outside of the ansible release process: +The following files are automatically generated and should not be modified outside of the Ansible release process: {% for file in changed_files %} - {{ file }} {% endfor %} -Please double check your changes. +Please double-check your changes.