This is an automated email from the ASF dual-hosted git repository.
davidarthur pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new 4dfea3c5f08 MINOR Improve PR linter output (#19159)
4dfea3c5f08 is described below
commit 4dfea3c5f0801b3b4a66e8917511074f883c6544
Author: David Arthur <[email protected]>
AuthorDate: Mon Mar 10 18:10:22 2025 -0400
MINOR Improve PR linter output (#19159)
Add a step summary for the PR linter which shows all the errors in a
more readable format.
Reviewers: Chia-Ping Tsai <[email protected]>
---
.github/scripts/pr-format.py | 94 ++++++++++++++++++++++-----------------
.github/workflows/README.md | 9 ++++
.github/workflows/pr-linter.yml | 2 +-
.github/workflows/pr-reviewed.yml | 2 +-
4 files changed, 64 insertions(+), 43 deletions(-)
diff --git a/.github/scripts/pr-format.py b/.github/scripts/pr-format.py
index fc661da0308..c71a19b4947 100644
--- a/.github/scripts/pr-format.py
+++ b/.github/scripts/pr-format.py
@@ -14,6 +14,7 @@
# limitations under the License.
from collections import defaultdict
+from io import BytesIO
import json
import logging
import os
@@ -21,7 +22,7 @@ import subprocess
import shlex
import sys
import tempfile
-from typing import Dict, Optional
+from typing import Dict, Optional, TextIO
logger = logging.getLogger()
@@ -30,6 +31,9 @@ handler = logging.StreamHandler(sys.stderr)
handler.setLevel(logging.DEBUG)
logger.addHandler(handler)
+ok = "✅"
+err = "❌"
+
def get_env(key: str, fn = str) -> Optional:
value = os.getenv(key)
@@ -42,23 +46,26 @@ def get_env(key: str, fn = str) -> Optional:
def has_approval(reviews) -> bool:
+ approved = False
for review in reviews:
- logger.debug(f"Review: {review}")
if review.get("authorAssociation") not in ("MEMBER", "OWNER"):
continue
if review.get("state") == "APPROVED":
- return True
- return False
+ approved = True
+ return approved
+
+def write_commit(io: TextIO, title: str, body: str):
+ io.write(title.encode())
+ io.write(b"\n\n")
+ io.write(body.encode())
+ io.flush()
def parse_trailers(title, body) -> Dict:
trailers = defaultdict(list)
with tempfile.NamedTemporaryFile() as fp:
- fp.write(title.encode())
- fp.write(b"\n")
- fp.write(body.encode())
- fp.flush()
+ write_commit(fp, title, body)
cmd = f"git interpret-trailers --trim-empty --parse {fp.name}"
p = subprocess.run(shlex.split(cmd), capture_output=True)
fp.close()
@@ -105,50 +112,55 @@ if __name__ == "__main__":
body = gh_json["body"]
reviews = gh_json["reviews"]
- warnings = []
- errors = []
-
- # Check title
- if title.endswith("..."):
- errors.append("Title appears truncated")
-
- if len(title) < 15:
- errors.append("Title is too short")
+ checks = [] # (bool (0=ok, 1=error), message)
- if len(title) > 120:
- errors.append("Title is too long")
+ def check(positive_assertion, ok_msg, err_msg):
+ if positive_assertion:
+ checks.append((0, f"{ok} {ok_msg}"))
+ else:
+ checks.append((1, f"{err} {err_msg}"))
- if not title.startswith("KAFKA-") and not title.startswith("MINOR") and
not title.startswith("HOTFIX"):
- errors.append("Title is missing KAFKA-XXXXX or MINOR/HOTFIX prefix")
+ # Check title
+ check(not title.endswith("..."), "Title is not truncated", "Title appears
truncated (ends with ...)")
+ check(len(title) >= 15, "Title is not too short", "Title is too short
(under 15 characters)")
+ check(len(title) <= 120, "Title is not too long", "Title is too long (over
120 characters)")
+ ok_prefix = title.startswith("KAFKA-") or title.startswith("MINOR") or
title.startswith("HOTFIX")
+ check(ok_prefix, "Title has expected KAFKA/MINOR/HOTFIX", "Title is
missing KAFKA-XXXXX or MINOR/HOTFIX prefix")
# Check body
- if len(body) == 0:
- errors.append("Body is empty")
-
- if "Delete this text and replace" in body:
- errors.append("PR template text should be removed")
+ check(len(body) != 0, "Body is not empty", "Body is empty")
+ check("Delete this text and replace" not in body, "PR template text not
present", "PR template text should be removed")
+ check("Committer Checklist" not in body, "PR template text not present",
"Old PR template text should be removed")
# Check for Reviewers
approved = has_approval(reviews)
if approved:
trailers = parse_trailers(title, body)
reviewers_in_body = trailers.get("Reviewers", [])
+ check(len(reviewers_in_body) > 0, "Found 'Reviewers' in commit body",
"Pull Request is approved, but no 'Reviewers' found in commit body")
if len(reviewers_in_body) > 0:
- logger.debug(f"Found 'Reviewers' in commit body")
for reviewer_in_body in reviewers_in_body:
logger.debug(reviewer_in_body)
- else:
- errors.append("Pull Request is approved, but no 'Reviewers' found
in commit body")
- for warning in warnings:
- logger.debug(warning)
-
- if len(errors) > 0:
- for error in errors:
- logger.debug(error)
- # Just output the first error for the status message
- print(errors[0])
- exit(1)
- else:
- print("PR format looks good!")
- exit(0)
+ logger.debug("Commit will look like:\n")
+ logger.debug("```")
+ io = BytesIO()
+ write_commit(io, title, body)
+ io.seek(0)
+ logger.debug(io.read().decode())
+ logger.debug("```\n")
+
+ exit_code = 0
+ logger.debug("Validation results:")
+ for err, msg in checks:
+ logger.debug(f"* {msg}")
+
+ for err, msg in checks:
+ # Just output the first error for the status message. STDOUT becomes
the status check message
+ if err:
+ print(msg)
+ exit(1)
+
+ logger.debug("No validation errors, PR format looks good!")
+ print("PR format looks good!")
+ exit(0)
diff --git a/.github/workflows/README.md b/.github/workflows/README.md
index f0beb3f3f71..229b72596ac 100644
--- a/.github/workflows/README.md
+++ b/.github/workflows/README.md
@@ -122,6 +122,15 @@ structure of the PR
Note that the pr-reviewed.yml workflow uses the `ci-approved` mechanism
described
above.
+The following checks are performed on our PRs:
+* Title is not too short or too long
+* Title starts with "KAFKA-", "MINOR", or "HOTFIX"
+* Body is not empty
+* Body includes "Reviewers:" if the PR is approved
+
+With the merge queue, our PR title and body will become the commit subject and
message.
+This linting step will help to ensure that we have nice looking commits.
+
### Stale PRs
This one is straightforward. Using the "actions/stale" GitHub Action, we
automatically
diff --git a/.github/workflows/pr-linter.yml b/.github/workflows/pr-linter.yml
index 43a3417ce4c..f19efbfabe2 100644
--- a/.github/workflows/pr-linter.yml
+++ b/.github/workflows/pr-linter.yml
@@ -61,7 +61,7 @@ jobs:
echo "Restored PR_NUMBER.txt:"
cat PR_NUMBER.txt
PR_NUMBER=$(cat PR_NUMBER.txt)
- PR_NUMBER=$PR_NUMBER python .github/scripts/pr-format.py >
pr-format-output.txt
+ PR_NUMBER=$PR_NUMBER python .github/scripts/pr-format.py 2>>
"$GITHUB_STEP_SUMMARY" 1>> pr-format-output.txt
exitcode="$?"
message=$(cat pr-format-output.txt)
echo "message=$message" >> "$GITHUB_OUTPUT"
diff --git a/.github/workflows/pr-reviewed.yml
b/.github/workflows/pr-reviewed.yml
index 46e9b3de915..64636a6c292 100644
--- a/.github/workflows/pr-reviewed.yml
+++ b/.github/workflows/pr-reviewed.yml
@@ -21,7 +21,7 @@ on:
branches:
- trunk
pull_request:
- types: [edited]
+ types: [opened, reopened, edited]
branches:
- trunk
