ci: improve template comments so contributors don't delete the marker.
This commit is contained in:
@@ -1,6 +1,12 @@
|
|||||||
<!-- noctalia-pr-template:v1 -->
|
<!-- noctalia-pr-template:v1 -->
|
||||||
|
<!-- ^ Keep the marker line above this comment.
|
||||||
|
|
||||||
<!-- If this PR is not ready for review yet, please mark it as Draft. -->
|
A bot closes pull requests whose description loses the marker line, a "##" heading,
|
||||||
|
a "- **Field:**" line, or a "- [ ]" checklist entry. Fill those in, do not delete them.
|
||||||
|
Everything else, including every one of these guidance comments, is yours to delete.
|
||||||
|
|
||||||
|
Not ready for review yet? Mark the pull request as Draft; checklist boxes may stay
|
||||||
|
unchecked while it is a draft. -->
|
||||||
|
|
||||||
## Plugin
|
## Plugin
|
||||||
|
|
||||||
|
|||||||
@@ -46,11 +46,25 @@ REQUIRED_CHECKLIST_ITEMS = (
|
|||||||
"Every network call, filesystem write, and spawned process is something the description above accounts for.",
|
"Every network call, filesystem write, and spawned process is something the description above accounts for.",
|
||||||
"I have the right to publish this code under the `license` declared in `plugin.toml`.",
|
"I have the right to publish this code under the `license` declared in `plugin.toml`.",
|
||||||
)
|
)
|
||||||
CLOSURE_COMMENT = f"""{COMMENT_MARKER}
|
CLOSURE_INTRO = f"""{COMMENT_MARKER}
|
||||||
This pull request was automatically closed because its description removed or altered required sections of the repository's pull request template.
|
This pull request was automatically closed because its description no longer contains
|
||||||
|
every part of [the pull request template](https://github.com/noctalia-dev/community-plugins/blob/main/.github/PULL_REQUEST_TEMPLATE.md)
|
||||||
|
that this repository requires.
|
||||||
|
|
||||||
Restore the current contents of `.github/PULL_REQUEST_TEMPLATE.md`, complete it without deleting its sections, fields, or checklist entries, and then reopen the pull request. Checklist boxes may remain unchecked while the pull request is a draft.
|
Missing:
|
||||||
"""
|
"""
|
||||||
|
CLOSURE_OUTRO = """
|
||||||
|
Please add the items listed above back to the description, keeping their exact wording, then
|
||||||
|
reopen the pull request. Reopening re-runs this check. Only the marker line, the `##` headings,
|
||||||
|
the `- **Field:**` lines, and the `- [ ]` checklist entries are required; the guidance comments
|
||||||
|
in the template are yours to delete, and checklist boxes may stay unchecked while the pull
|
||||||
|
request is a draft.
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
def build_closure_comment(missing: list[str]) -> str:
|
||||||
|
bullets = "".join(f"- {item}\n" for item in missing)
|
||||||
|
return f"{CLOSURE_INTRO}{bullets}{CLOSURE_OUTRO}"
|
||||||
|
|
||||||
|
|
||||||
def missing_requirements(body: object) -> list[str]:
|
def missing_requirements(body: object) -> list[str]:
|
||||||
@@ -61,23 +75,23 @@ def missing_requirements(body: object) -> list[str]:
|
|||||||
normalized_body = " ".join(body.split())
|
normalized_body = " ".join(body.split())
|
||||||
missing: list[str] = []
|
missing: list[str] = []
|
||||||
|
|
||||||
if TEMPLATE_MARKER not in lines:
|
if TEMPLATE_MARKER not in normalized_body:
|
||||||
missing.append("template version marker")
|
missing.append(f"the template marker line `{TEMPLATE_MARKER}`")
|
||||||
|
|
||||||
for heading in REQUIRED_HEADINGS:
|
for heading in REQUIRED_HEADINGS:
|
||||||
if heading not in lines:
|
if heading not in lines:
|
||||||
missing.append(f"{heading} section")
|
missing.append(f"the `{heading}` heading")
|
||||||
|
|
||||||
for prefix in REQUIRED_FIELD_PREFIXES:
|
for prefix in REQUIRED_FIELD_PREFIXES:
|
||||||
if prefix not in normalized_body:
|
if prefix not in normalized_body:
|
||||||
missing.append(f"field: {prefix.removeprefix('- ')}")
|
missing.append(f"the `{prefix}` field")
|
||||||
|
|
||||||
for item in REQUIRED_CHECKLIST_ITEMS:
|
for item in REQUIRED_CHECKLIST_ITEMS:
|
||||||
if not any(
|
if not any(
|
||||||
f"- [{state}] {item}" in normalized_body
|
f"- [{state}] {item}" in normalized_body
|
||||||
for state in (" ", "x", "X")
|
for state in (" ", "x", "X")
|
||||||
):
|
):
|
||||||
missing.append(f"checklist item: {item}")
|
missing.append(f"the checklist entry: {item}")
|
||||||
|
|
||||||
return missing
|
return missing
|
||||||
|
|
||||||
@@ -107,8 +121,9 @@ def github_request(
|
|||||||
return json.loads(response_body) if response_body else None
|
return json.loads(response_body) if response_body else None
|
||||||
|
|
||||||
|
|
||||||
def has_enforcement_comment(issue_url: str, token: str) -> bool:
|
def latest_enforcement_comment(issue_url: str, token: str) -> str | None:
|
||||||
page = 1
|
page = 1
|
||||||
|
latest: str | None = None
|
||||||
while True:
|
while True:
|
||||||
comments = github_request(
|
comments = github_request(
|
||||||
f"{issue_url}/comments?per_page=100&page={page}",
|
f"{issue_url}/comments?per_page=100&page={page}",
|
||||||
@@ -116,14 +131,14 @@ def has_enforcement_comment(issue_url: str, token: str) -> bool:
|
|||||||
)
|
)
|
||||||
if not isinstance(comments, list):
|
if not isinstance(comments, list):
|
||||||
raise RuntimeError("GitHub returned an invalid pull request comment list")
|
raise RuntimeError("GitHub returned an invalid pull request comment list")
|
||||||
if any(
|
for comment in comments:
|
||||||
isinstance(comment, dict)
|
if not isinstance(comment, dict):
|
||||||
and COMMENT_MARKER in str(comment.get("body", ""))
|
continue
|
||||||
for comment in comments
|
body = str(comment.get("body", ""))
|
||||||
):
|
if COMMENT_MARKER in body:
|
||||||
return True
|
latest = body
|
||||||
if len(comments) < 100:
|
if len(comments) < 100:
|
||||||
return False
|
return latest
|
||||||
page += 1
|
page += 1
|
||||||
|
|
||||||
|
|
||||||
@@ -143,12 +158,13 @@ def enforce(event: dict[str, object], token: str) -> list[str]:
|
|||||||
if not token:
|
if not token:
|
||||||
raise ValueError("GITHUB_TOKEN is required to close an invalid pull request")
|
raise ValueError("GITHUB_TOKEN is required to close an invalid pull request")
|
||||||
|
|
||||||
if not has_enforcement_comment(issue_url, token):
|
comment = build_closure_comment(missing)
|
||||||
|
if latest_enforcement_comment(issue_url, token) != comment:
|
||||||
github_request(
|
github_request(
|
||||||
f"{issue_url}/comments",
|
f"{issue_url}/comments",
|
||||||
token,
|
token,
|
||||||
method="POST",
|
method="POST",
|
||||||
payload={"body": CLOSURE_COMMENT},
|
payload={"body": comment},
|
||||||
)
|
)
|
||||||
github_request(
|
github_request(
|
||||||
pull_request_url,
|
pull_request_url,
|
||||||
@@ -171,7 +187,10 @@ def main(argv: list[str]) -> int:
|
|||||||
return 1
|
return 1
|
||||||
|
|
||||||
if missing:
|
if missing:
|
||||||
print("::error title=Required PR template content is missing::" + "; ".join(missing))
|
print(
|
||||||
|
"::error title=Pull request description is missing required template content::"
|
||||||
|
+ "; ".join(missing)
|
||||||
|
)
|
||||||
return 1
|
return 1
|
||||||
|
|
||||||
print("Pull request description retains the required template structure.")
|
print("Pull request description retains the required template structure.")
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import importlib.util
|
import importlib.util
|
||||||
|
import re
|
||||||
import unittest
|
import unittest
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
@@ -33,25 +34,36 @@ class TemplateValidationTests(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertEqual(enforce_pr_template.missing_requirements(wrapped), [])
|
self.assertEqual(enforce_pr_template.missing_requirements(wrapped), [])
|
||||||
|
|
||||||
|
def test_accepts_body_stripped_of_guidance_comments(self) -> None:
|
||||||
|
stripped = re.sub(
|
||||||
|
r"<!--(?!\s*noctalia-pr-template:v1\s*-->).*?-->",
|
||||||
|
"",
|
||||||
|
self.template,
|
||||||
|
flags=re.DOTALL,
|
||||||
|
)
|
||||||
|
self.assertNotIn("guidance", stripped)
|
||||||
|
self.assertIn(enforce_pr_template.TEMPLATE_MARKER, stripped)
|
||||||
|
self.assertEqual(enforce_pr_template.missing_requirements(stripped), [])
|
||||||
|
|
||||||
def test_rejects_missing_version_marker(self) -> None:
|
def test_rejects_missing_version_marker(self) -> None:
|
||||||
body = self.template.replace(enforce_pr_template.TEMPLATE_MARKER, "")
|
body = self.template.replace(enforce_pr_template.TEMPLATE_MARKER, "")
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
enforce_pr_template.missing_requirements(body),
|
enforce_pr_template.missing_requirements(body),
|
||||||
["template version marker"],
|
["the template marker line `<!-- noctalia-pr-template:v1 -->`"],
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_rejects_removed_section(self) -> None:
|
def test_rejects_removed_section(self) -> None:
|
||||||
body = self.template.replace("## Testing", "## Verification")
|
body = self.template.replace("## Testing", "## Verification")
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
enforce_pr_template.missing_requirements(body),
|
enforce_pr_template.missing_requirements(body),
|
||||||
["## Testing section"],
|
["the `## Testing` heading"],
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_rejects_removed_required_field(self) -> None:
|
def test_rejects_removed_required_field(self) -> None:
|
||||||
body = self.template.replace("- **Plugin API level:**", "- **API:**")
|
body = self.template.replace("- **Plugin API level:**", "- **API:**")
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
enforce_pr_template.missing_requirements(body),
|
enforce_pr_template.missing_requirements(body),
|
||||||
["field: **Plugin API level:**"],
|
["the `- **Plugin API level:**` field"],
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_rejects_altered_multiline_checklist_item(self) -> None:
|
def test_rejects_altered_multiline_checklist_item(self) -> None:
|
||||||
@@ -62,7 +74,7 @@ class TemplateValidationTests(unittest.TestCase):
|
|||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
enforce_pr_template.missing_requirements(body),
|
enforce_pr_template.missing_requirements(body),
|
||||||
[
|
[
|
||||||
"checklist item: `README.md` follows the "
|
"the checklist entry: `README.md` follows the "
|
||||||
"[README template](https://github.com/noctalia-dev/community-plugins/blob/main/README_TEMPLATE.md), "
|
"[README template](https://github.com/noctalia-dev/community-plugins/blob/main/README_TEMPLATE.md), "
|
||||||
"documents every entry id and dependency, and includes exact panel IPC commands and launcher prefixes where applicable."
|
"documents every entry id and dependency, and includes exact panel IPC commands and launcher prefixes where applicable."
|
||||||
],
|
],
|
||||||
@@ -102,7 +114,13 @@ class TemplateEnforcementTests(unittest.TestCase):
|
|||||||
"token",
|
"token",
|
||||||
)
|
)
|
||||||
|
|
||||||
self.assertIn("template version marker", missing)
|
self.assertIn(
|
||||||
|
"the template marker line `<!-- noctalia-pr-template:v1 -->`",
|
||||||
|
missing,
|
||||||
|
)
|
||||||
|
comment = enforce_pr_template.build_closure_comment(missing)
|
||||||
|
for item in missing:
|
||||||
|
self.assertIn(f"- {item}\n", comment)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
request.call_args_list,
|
request.call_args_list,
|
||||||
[
|
[
|
||||||
@@ -114,7 +132,7 @@ class TemplateEnforcementTests(unittest.TestCase):
|
|||||||
f"{self.ISSUE_URL}/comments",
|
f"{self.ISSUE_URL}/comments",
|
||||||
"token",
|
"token",
|
||||||
method="POST",
|
method="POST",
|
||||||
payload={"body": enforce_pr_template.CLOSURE_COMMENT},
|
payload={"body": comment},
|
||||||
),
|
),
|
||||||
mock.call(
|
mock.call(
|
||||||
self.PULL_REQUEST_URL,
|
self.PULL_REQUEST_URL,
|
||||||
@@ -125,17 +143,16 @@ class TemplateEnforcementTests(unittest.TestCase):
|
|||||||
],
|
],
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_existing_enforcement_comment_is_not_duplicated(self) -> None:
|
def test_identical_enforcement_comment_is_not_duplicated(self) -> None:
|
||||||
existing_comment = {"body": enforce_pr_template.CLOSURE_COMMENT}
|
body = "AI-generated replacement body"
|
||||||
|
missing = enforce_pr_template.missing_requirements(body)
|
||||||
|
existing_comment = {"body": enforce_pr_template.build_closure_comment(missing)}
|
||||||
with mock.patch.object(
|
with mock.patch.object(
|
||||||
enforce_pr_template,
|
enforce_pr_template,
|
||||||
"github_request",
|
"github_request",
|
||||||
side_effect=[[existing_comment], {}],
|
side_effect=[[existing_comment], {}],
|
||||||
) as request:
|
) as request:
|
||||||
enforce_pr_template.enforce(
|
enforce_pr_template.enforce(self.event(body), "token")
|
||||||
self.event("AI-generated replacement body"),
|
|
||||||
"token",
|
|
||||||
)
|
|
||||||
|
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
request.call_args_list,
|
request.call_args_list,
|
||||||
@@ -153,6 +170,31 @@ class TemplateEnforcementTests(unittest.TestCase):
|
|||||||
],
|
],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_stale_enforcement_comment_is_replaced_with_current_findings(self) -> None:
|
||||||
|
body = TEMPLATE_PATH.read_text().replace("## Testing", "## Verification")
|
||||||
|
stale = {
|
||||||
|
"body": enforce_pr_template.build_closure_comment(
|
||||||
|
["the template marker line `<!-- noctalia-pr-template:v1 -->`"]
|
||||||
|
)
|
||||||
|
}
|
||||||
|
with mock.patch.object(
|
||||||
|
enforce_pr_template,
|
||||||
|
"github_request",
|
||||||
|
side_effect=[[stale], {}, {}],
|
||||||
|
) as request:
|
||||||
|
missing = enforce_pr_template.enforce(self.event(body), "token")
|
||||||
|
|
||||||
|
self.assertEqual(missing, ["the `## Testing` heading"])
|
||||||
|
self.assertEqual(
|
||||||
|
request.call_args_list[1],
|
||||||
|
mock.call(
|
||||||
|
f"{self.ISSUE_URL}/comments",
|
||||||
|
"token",
|
||||||
|
method="POST",
|
||||||
|
payload={"body": enforce_pr_template.build_closure_comment(missing)},
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user