-
Notifications
You must be signed in to change notification settings - Fork 6
Move template share prompt into review body - no more issues #254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
trexfeathers
wants to merge
10
commits into
SciTools:main
Choose a base branch
from
trexfeathers:no-more-issues
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
32eea98
Post all templating suggestions directly in a review, no new issue.
trexfeathers 5b50b93
Allow for more zealous templating suggestions around docs.
trexfeathers 82cd224
Case insensitive MAGIC phrase matching.
trexfeathers 349014c
Clickable links for changed paths.
trexfeathers f13df50
Avoid spamming multiple reviews.
trexfeathers ecc4ceb
Corrections from testing.
trexfeathers 7a7d0bd
Include an assignee for any issues raised.
trexfeathers 5e89f76
Remove debugging line.
trexfeathers 62c12ca
Use TEMPLATING_HEADING constant.
trexfeathers 0b2247c
Add scitools-ci to the BOTS list.
trexfeathers File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ | |
| """ | ||
| import argparse | ||
| import contextlib | ||
| from enum import StrEnum | ||
| from hashlib import sha256 | ||
| import json | ||
| from pathlib import Path | ||
| import re | ||
|
|
@@ -24,8 +26,14 @@ | |
| 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): | ||
| APPROVE = "approve" | ||
| COMMENT = "comment" | ||
| REQUEST_CHANGES = "request-changes" | ||
|
|
||
|
|
||
| def git_command(command: str) -> str: | ||
|
|
@@ -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,70 +250,38 @@ 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, " | ||
| "which would not require any templating follow-up. **Please double-" | ||
| "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." | ||
| ) | ||
|
|
@@ -337,15 +306,20 @@ def create_issue(title: str, body: str) -> 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 = ( | ||
| 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}" | ||
| f"- [ ] {changed_link}, templated by {template_link}" | ||
| ) | ||
|
|
||
| else: | ||
|
|
@@ -359,13 +333,9 @@ def create_issue(title: str, body: str) -> None: | |
| if changed_parent in ( | ||
| git_root, | ||
| git_root / "benchmarks", | ||
| git_root / "docs" / "src", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Going back to this given there will no longer be annoying issues raised every time. (originally removed in #218) |
||
| ): | ||
| candidates_list.append(f"- [ ] `{changed_path}`") | ||
| if changed_path in ( | ||
| git_root / "docs" / "src" / "conf.py", | ||
| git_root / "docs" / "src" / "Makefile", | ||
| ): | ||
| candidates_list.append(f"- [ ] `{changed_path}`") | ||
| candidates_list.append(f"- [ ] {changed_link}") | ||
|
|
||
| if templated_list or candidates_list: | ||
| body_args = [body_intro] | ||
|
|
@@ -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: | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.