From 32eea984cebfe2fb9d0f36070e65ec966ccde9ab Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Jan 2026 18:07:24 +0000 Subject: [PATCH 01/10] Post all templating suggestions directly in a review, no new issue. --- templates/_templating_scripting.py | 82 ++++++++++-------------------- 1 file changed, 26 insertions(+), 56 deletions(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index de0a3a7..2bd19e7 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -3,6 +3,7 @@ """ import argparse import contextlib +from enum import StrEnum import json from pathlib import Path import re @@ -28,6 +29,12 @@ MAGIC_NO_NOTIFY = re.compile(rf"{_MAGIC_PREFIX} no update notification on: ([\w-]+)") +class ReviewType(StrEnum): + APPROVE = "approve" + COMMENT = "comment" + REQUEST_CHANGES = "request-changes" + + def git_command(command: str) -> str: command = shlex.split(f"git {command}") return check_output(command).decode("utf-8").strip() @@ -209,6 +216,7 @@ def prompt_share(args: argparse.Namespace) -> None: # pr_number = "https://github.com/SciTools/iris/pull/6496" body = gh_json(f"pr view {pr_number}", "body")["body"] + # TODO: make this case-insensitive. if MAGIC_NO_PROMPT.search(body): print( f"Skipping PR {pr_number} because the body contains the magic " @@ -220,16 +228,9 @@ def split_github_url(url: str) -> tuple[str, str, str]: _, org, repo, _, ref = urlparse(url).path.split("/") return org, repo, ref - def url_to_short_ref(url: str) -> str: - org, repo, ref = split_github_url(url) - return f"{org}/{repo}#{ref}" - pr_url = gh_json(f"pr view {pr_number}", "url")["url"] - pr_short_ref = url_to_short_ref(pr_url) pr_repo = split_github_url(pr_url)[1] - author = gh_json(f"pr view {pr_number}", "author")["author"]["login"] - changed_files = gh_json(f"pr view {pr_number}", "files")["files"] changed_paths = [Path(file["path"]) for file in changed_files] @@ -249,9 +250,19 @@ def get_all_authors() -> set[str]: for commit_author in get_commit_authors(commit) ) + def post_review(review_body: str, review_type: ReviewType) -> None: + with NamedTemporaryFile("w") as file_write: + file_write.write(review_body) + file_write.flush() + gh_command = shlex.split( + f"gh pr review {pr_number} --{review_type.value} " + f"--body-file {file_write.name}" + ) + run(gh_command, check=True) + human_authors = get_all_authors() - set(BOTS) if human_authors == set(): - review_body = ( + review_text = ( f"### [Templating]({SCITOOLS_URL}/.github/blob/main/templates)\n\n" "Version numbers are not typically covered by templating. It is " "expected that this PR is 100% about advancing version numbers, " @@ -259,60 +270,18 @@ def get_all_authors() -> set[str]: "check for any other changes that might be suitable for " "templating**." ) - with NamedTemporaryFile("w") as file_write: - file_write.write(review_body) - file_write.flush() - gh_command = shlex.split( - f"gh pr review {pr_number} --comment --body-file {file_write.name}" - ) - run(gh_command, check=True) + post_review(review_text, ReviewType.COMMENT) return - def create_issue(title: str, body: str) -> None: - assignee = author - - # Check that an issue with this title isn't already on the .github repo. - existing_issues = gh_json( - "issue list --state all --repo SciTools/.github", "title" - ) - if any(issue["title"] == title for issue in existing_issues): - return - - if assignee in BOTS: - # if the author is a bot, we don't want to assign the issue to the bot - # so instead choose a human author from the latest commit - assignee = list(human_authors)[0] - - with NamedTemporaryFile("w") as file_write: - file_write.write(body) - file_write.flush() - gh_command = shlex.split( - "gh issue create " - f'--title "{title}" ' - f"--body-file {file_write.name} " - "--repo SciTools/.github " - f"--assignee {assignee}" - ) - issue_url = check_output(gh_command).decode("utf-8").strip() - short_ref = url_to_short_ref(issue_url) - # GitHub renders the full text of a cross-ref when it is in a list. - review_body = f"- [ ] Please see: {short_ref}" - gh_command = shlex.split( - f'gh pr review {pr_number} --request-changes --body "{review_body}"' - ) - run(gh_command, check=True) - - issue_title = f"Share {pr_short_ref} changes via templating?" - templates_relative = TEMPLATES_DIR.relative_to(TEMPLATE_REPO_ROOT) templates_url = f"{SCITOOLS_URL}/.github/tree/main/{templates_relative}" body_intro = ( f"## [Templating]({SCITOOLS_URL}/.github/blob/main/templates/README.md)\n\n" - f"{pr_short_ref} (by @{author}) includes changes that may be worth " + f"This PR includes changes that may be worth " "sharing via templating. For each file listed below, please " "either:\n\n" "- Action the suggestion via a pull request editing/adding the " - f"relevant file in the [templates directory]({templates_url}). [^1]\n" + f"relevant file in the [SciTools/.github `templates/` directory]({templates_url}). [^1]\n" "- Dismiss the suggestion if the changes are not suitable for " "templating." ) @@ -342,7 +311,7 @@ def create_issue(title: str, body: str) -> None: template_url = ( f"{SCITOOLS_URL}/.github/blob/main/{template_relative}" ) - template_link = f"[`{template_relative}`]({template_url})" + template_link = f"[`SciTools/.github/{template_relative}`]({template_url})" templated_list.append( f"- [ ] `{changed_path}`, templated by {template_link}" @@ -361,6 +330,7 @@ def create_issue(title: str, body: str) -> None: git_root / "benchmarks", ): candidates_list.append(f"- [ ] `{changed_path}`") + # TODO: bring back more generic docs location. if changed_path in ( git_root / "docs" / "src" / "conf.py", git_root / "docs" / "src" / "Makefile", @@ -385,8 +355,8 @@ def create_issue(title: str, body: str) -> None: f"``{pattern_repo}``" ) - issue_body = "\n".join(body_args) - create_issue(issue_title, issue_body) + review_text= "\n".join(body_args) + post_review(review_text, ReviewType.REQUEST_CHANGES) def check_dir(args: argparse.Namespace) -> None: From 5b50b934e1dc9d82f6041cec9ff28f0c8d12c8d9 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Jan 2026 18:09:02 +0000 Subject: [PATCH 02/10] Allow for more zealous templating suggestions around docs. --- templates/_templating_scripting.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index 2bd19e7..6601702 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -328,12 +328,7 @@ def post_review(review_body: str, review_type: ReviewType) -> None: if changed_parent in ( git_root, git_root / "benchmarks", - ): - candidates_list.append(f"- [ ] `{changed_path}`") - # TODO: bring back more generic docs location. - if changed_path in ( - git_root / "docs" / "src" / "conf.py", - git_root / "docs" / "src" / "Makefile", + git_root / "docs" / "src", ): candidates_list.append(f"- [ ] `{changed_path}`") From 82cd22470f32f78afb8174a8ce04d02e7749c369 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Jan 2026 18:10:10 +0000 Subject: [PATCH 03/10] Case insensitive MAGIC phrase matching. --- templates/_templating_scripting.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index 6601702..2673f6d 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -25,8 +25,8 @@ BOTS = ["dependabot[bot]", "app/dependabot", "pre-commit-ci[bot]", "app/pre-commit-ci"] _MAGIC_PREFIX = "@scitools-templating: please" -MAGIC_NO_PROMPT = re.compile(rf"{_MAGIC_PREFIX} no share prompt") -MAGIC_NO_NOTIFY = re.compile(rf"{_MAGIC_PREFIX} no update notification on: ([\w-]+)") +MAGIC_NO_PROMPT = re.compile(rf"{_MAGIC_PREFIX} no share prompt", re.IGNORECASE) +MAGIC_NO_NOTIFY = re.compile(rf"{_MAGIC_PREFIX} no update notification on: ([\w-]+)", re.IGNORECASE) class ReviewType(StrEnum): @@ -216,7 +216,6 @@ def prompt_share(args: argparse.Namespace) -> None: # pr_number = "https://github.com/SciTools/iris/pull/6496" body = gh_json(f"pr view {pr_number}", "body")["body"] - # TODO: make this case-insensitive. if MAGIC_NO_PROMPT.search(body): print( f"Skipping PR {pr_number} because the body contains the magic " From 349014c110ef34424f01c0f1240bbb5d53aaf98f Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Tue, 27 Jan 2026 18:34:06 +0000 Subject: [PATCH 04/10] Clickable links for changed paths. --- templates/_templating_scripting.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index 2673f6d..313681b 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -4,6 +4,7 @@ import argparse import contextlib from enum import StrEnum +from hashlib import sha256 import json from pathlib import Path import re @@ -305,6 +306,11 @@ def post_review(review_body: str, review_type: ReviewType) -> None: ignored = str(changed_path) in ignore_dict[pr_repo] if ignored: continue + + changed_hash = sha256(str(changed_path).encode()).hexdigest() + changed_url = f"{pr_url}/files#diff-{changed_hash}" + changed_link = f"[`{changed_path}`]({changed_url})" + if is_templated: template_relative = template.relative_to(TEMPLATE_REPO_ROOT) template_url = ( @@ -313,7 +319,7 @@ def post_review(review_body: str, review_type: ReviewType) -> None: template_link = f"[`SciTools/.github/{template_relative}`]({template_url})" templated_list.append( - f"- [ ] `{changed_path}`, templated by {template_link}" + f"- [ ] {changed_link}, templated by {template_link}" ) else: @@ -329,7 +335,7 @@ def post_review(review_body: str, review_type: ReviewType) -> None: git_root / "benchmarks", git_root / "docs" / "src", ): - candidates_list.append(f"- [ ] `{changed_path}`") + candidates_list.append(f"- [ ] {changed_link}") if templated_list or candidates_list: body_args = [body_intro] From f13df509b194b5ea52fd3288ce8f249ddbd62b17 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Wed, 28 Jan 2026 14:43:13 +0000 Subject: [PATCH 05/10] Avoid spamming multiple reviews. --- templates/_templating_scripting.py | 35 ++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index 313681b..e3ac546 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -11,7 +11,7 @@ import shlex from subprocess import CalledProcessError, check_output, run from tempfile import NamedTemporaryFile -from typing import NamedTuple +from typing import NamedTuple, Optional from urllib.parse import urlparse # A mechanism for disabling the issues and comments if the dev team is @@ -41,9 +41,11 @@ def git_command(command: str) -> str: return check_output(command).decode("utf-8").strip() -def gh_json(sub_command: str, field: str) -> dict: - command = shlex.split(f"gh {sub_command} --json {field}") - return json.loads(check_output(command)) +def gh_json(sub_command: str, field: Optional[str] = None) -> dict: + command = f"gh {sub_command}" + if field: + command += f" --json {field}" + return json.loads(check_output(shlex.split(command))) class Config: @@ -216,6 +218,8 @@ def prompt_share(args: argparse.Namespace) -> None: # Can use a URL here for local debugging: # pr_number = "https://github.com/SciTools/iris/pull/6496" + current_user = gh_json("api user")["login"] + body = gh_json(f"pr view {pr_number}", "body")["body"] if MAGIC_NO_PROMPT.search(body): print( @@ -251,13 +255,21 @@ def get_all_authors() -> set[str]: ) def post_review(review_body: str, review_type: ReviewType) -> None: + # Check for any existing reviews by this user. + # If so: we will just edit the most recent review. + reviews = gh_json(f"pr view {pr_number}", "reviews")["reviews"] + has_existing_review = any( + review["author"]["login"] == current_user for review in reviews + ) + with NamedTemporaryFile("w") as file_write: file_write.write(review_body) file_write.flush() - gh_command = shlex.split( - f"gh pr review {pr_number} --{review_type.value} " - f"--body-file {file_write.name}" - ) + if has_existing_review: + command = f"gh pr comment {pr_number} --edit-last" + else: + command = f"gh pr review {pr_number} --{review_type.value}" + gh_command = shlex.split(f"{command} --body-file {file_write.name}") run(gh_command, check=True) human_authors = get_all_authors() - set(BOTS) @@ -282,8 +294,13 @@ def post_review(review_body: str, review_type: ReviewType) -> None: "either:\n\n" "- Action the suggestion via a pull request editing/adding the " f"relevant file in the [SciTools/.github `templates/` directory]({templates_url}). [^1]\n" + "- Raise an issue against the [SciTools/.github repo]({SCITOOLS_URL}/.github) " + "for the above action if you _really_ don't have 10mins spare right now.\n" "- Dismiss the suggestion if the changes are not suitable for " - "templating." + "templating.\n\n" + "You will need to dismiss this review before this PR can be merged. " + "*Recommend the reviewer does this as their final action before " + "merging*, as this text will continually update as commits come in." ) templated_list = [] From ecc4ceb12f5b06fe3027e127b7e3a379b79ea28b Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Wed, 28 Jan 2026 17:04:58 +0000 Subject: [PATCH 06/10] Corrections from testing. --- templates/_templating_scripting.py | 58 +++++++++++++++++++----------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index e3ac546..2dd3d05 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -216,7 +216,7 @@ def prompt_share(args: argparse.Namespace) -> None: pr_number = args.pr_number # Can use a URL here for local debugging: - # pr_number = "https://github.com/SciTools/iris/pull/6496" + pr_number = "https://github.com/SciTools/iris/pull/6901" current_user = gh_json("api user")["login"] @@ -255,27 +255,45 @@ def get_all_authors() -> set[str]: ) def post_review(review_body: str, review_type: ReviewType) -> None: - # Check for any existing reviews by this user. - # If so: we will just edit the most recent review. - reviews = gh_json(f"pr view {pr_number}", "reviews")["reviews"] - has_existing_review = any( - review["author"]["login"] == current_user for review in reviews - ) + pr_int = pr_number + if pr_int == pr_url: + # Sometimes happens during local debugging. + pr_int = gh_json(f"pr view {pr_number}", "number")["number"] + + # Find any existing templating reviews. Edit the last one if found. + gh_command = f"gh api repos/SciTools/{pr_repo}/pulls/{pr_int}/reviews" + existing_reviews = json.loads(check_output(shlex.split(gh_command))) + reviews_to_edit = [ + review for review in existing_reviews + if review["user"]["login"] == current_user + and review["body"].startswith("## [Templating") + ] + if reviews_to_edit: + # Edit the last existing review. + review = reviews_to_edit[-1] + payload = json.dumps({"body": review_body}) + gh_command = ( + f"gh api --method PUT " + f"repos/SciTools/{pr_repo}/pulls/{pr_int}/reviews/{review['id']} " + f"--input -" + ) + run(shlex.split(gh_command), input=payload.encode(), check=True) - with NamedTemporaryFile("w") as file_write: - file_write.write(review_body) - file_write.flush() - if has_existing_review: - command = f"gh pr comment {pr_number} --edit-last" - else: - command = f"gh pr review {pr_number} --{review_type.value}" - gh_command = shlex.split(f"{command} --body-file {file_write.name}") - run(gh_command, check=True) + else: + # Create a new review. + with NamedTemporaryFile("w") as file_write: + file_write.write(review_body) + file_write.flush() + gh_command = ( + f"gh pr review {pr_number} --{review_type.value} " + f"--body-file {file_write.name}" + ) + run(shlex.split(gh_command), check=True) human_authors = get_all_authors() - set(BOTS) if human_authors == set(): review_text = ( - f"### [Templating]({SCITOOLS_URL}/.github/blob/main/templates)\n\n" + f"## [Templating]({SCITOOLS_URL}/.github/blob/main/templates)\n\n" "Version numbers are not typically covered by templating. It is " "expected that this PR is 100% about advancing version numbers, " "which would not require any templating follow-up. **Please double-" @@ -294,13 +312,13 @@ def post_review(review_body: str, review_type: ReviewType) -> None: "either:\n\n" "- Action the suggestion via a pull request editing/adding the " f"relevant file in the [SciTools/.github `templates/` directory]({templates_url}). [^1]\n" - "- Raise an issue against the [SciTools/.github repo]({SCITOOLS_URL}/.github) " + f"- Raise an issue against the [SciTools/.github repo]({SCITOOLS_URL}/.github) " "for the above action if you _really_ don't have 10mins spare right now.\n" "- Dismiss the suggestion if the changes are not suitable for " "templating.\n\n" "You will need to dismiss this review before this PR can be merged. " - "*Recommend the reviewer does this as their final action before " - "merging*, as this text will continually update as commits come in." + "**Recommend the reviewer does this as their final action before " + "merging**, as this text will continually update as commits come in." ) templated_list = [] From 7a7d0bd7ffdca1f80f65cb141dcaa8b2a20edee3 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 29 Jan 2026 15:13:46 +0000 Subject: [PATCH 07/10] Include an assignee for any issues raised. --- templates/_templating_scripting.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index 2dd3d05..860bd3c 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -313,7 +313,8 @@ def post_review(review_body: str, review_type: ReviewType) -> None: "- Action the suggestion via a pull request editing/adding the " f"relevant file in the [SciTools/.github `templates/` directory]({templates_url}). [^1]\n" f"- Raise an issue against the [SciTools/.github repo]({SCITOOLS_URL}/.github) " - "for the above action if you _really_ don't have 10mins spare right now.\n" + "for the above action if you _really_ don't have 10mins spare right now. " + "**Include an assignee**, to avoid it being forgotten.\n" "- Dismiss the suggestion if the changes are not suitable for " "templating.\n\n" "You will need to dismiss this review before this PR can be merged. " From 5e89f765daafb8f61fd871210e9ddad5e28e9446 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 29 Jan 2026 15:16:52 +0000 Subject: [PATCH 08/10] Remove debugging line. --- templates/_templating_scripting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index 860bd3c..1a93a22 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -216,7 +216,7 @@ def prompt_share(args: argparse.Namespace) -> None: pr_number = args.pr_number # Can use a URL here for local debugging: - pr_number = "https://github.com/SciTools/iris/pull/6901" + # pr_number = "https://github.com/SciTools/iris/pull/6901" current_user = gh_json("api user")["login"] From 62c12cabb6af526e9cbba6dd13a31008139ab9c7 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 29 Jan 2026 15:46:42 +0000 Subject: [PATCH 09/10] Use TEMPLATING_HEADING constant. --- templates/_templating_scripting.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index 1a93a22..547b2e6 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -24,6 +24,7 @@ TEMPLATE_REPO_ROOT = TEMPLATES_DIR.parent # ensure any new bots have both a "app/" prefix and a "[bot]" postfix version BOTS = ["dependabot[bot]", "app/dependabot", "pre-commit-ci[bot]", "app/pre-commit-ci"] +TEMPLATING_HEADING = f"## [Templating]({SCITOOLS_URL}/.github/blob/main/templates)" _MAGIC_PREFIX = "@scitools-templating: please" MAGIC_NO_PROMPT = re.compile(rf"{_MAGIC_PREFIX} no share prompt", re.IGNORECASE) @@ -155,6 +156,8 @@ def git_diff(*args: str) -> str: file_url = f"{SCITOOLS_URL}/{repo}/blob/main/{path_in_repo}" file_link = f"[`{path_in_repo}`]({file_url})" issue_body = ( + f"{TEMPLATING_HEADING}\n\n" + f"The template for `{path_in_repo}` has been updated; see the " "diff below. Please either:\n\n" @@ -266,7 +269,7 @@ def post_review(review_body: str, review_type: ReviewType) -> None: reviews_to_edit = [ review for review in existing_reviews if review["user"]["login"] == current_user - and review["body"].startswith("## [Templating") + and review["body"].startswith(TEMPLATING_HEADING) ] if reviews_to_edit: # Edit the last existing review. @@ -293,7 +296,7 @@ def post_review(review_body: str, review_type: ReviewType) -> None: human_authors = get_all_authors() - set(BOTS) if human_authors == set(): review_text = ( - f"## [Templating]({SCITOOLS_URL}/.github/blob/main/templates)\n\n" + f"{TEMPLATING_HEADING}\n\n" "Version numbers are not typically covered by templating. It is " "expected that this PR is 100% about advancing version numbers, " "which would not require any templating follow-up. **Please double-" @@ -306,7 +309,7 @@ def post_review(review_body: str, review_type: ReviewType) -> None: templates_relative = TEMPLATES_DIR.relative_to(TEMPLATE_REPO_ROOT) templates_url = f"{SCITOOLS_URL}/.github/tree/main/{templates_relative}" body_intro = ( - f"## [Templating]({SCITOOLS_URL}/.github/blob/main/templates/README.md)\n\n" + f"{TEMPLATING_HEADING}\n\n" f"This PR includes changes that may be worth " "sharing via templating. For each file listed below, please " "either:\n\n" From 0b2247cf26e1b0bb169b83837f2c1a4e619f13e8 Mon Sep 17 00:00:00 2001 From: Martin Yeo Date: Thu, 29 Jan 2026 15:48:26 +0000 Subject: [PATCH 10/10] Add scitools-ci to the BOTS list. --- templates/_templating_scripting.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/templates/_templating_scripting.py b/templates/_templating_scripting.py index 547b2e6..1d3bf35 100755 --- a/templates/_templating_scripting.py +++ b/templates/_templating_scripting.py @@ -23,7 +23,13 @@ TEMPLATES_DIR = Path(__file__).parent.resolve() TEMPLATE_REPO_ROOT = TEMPLATES_DIR.parent # ensure any new bots have both a "app/" prefix and a "[bot]" postfix version -BOTS = ["dependabot[bot]", "app/dependabot", "pre-commit-ci[bot]", "app/pre-commit-ci"] +BOTS = [ + "dependabot[bot]", + "app/dependabot", + "pre-commit-ci[bot]", + "app/pre-commit-ci", + "app/scitools-ci", +] TEMPLATING_HEADING = f"## [Templating]({SCITOOLS_URL}/.github/blob/main/templates)" _MAGIC_PREFIX = "@scitools-templating: please"