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 <chia7712@gmail.com>

.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 = []
+    checks = [] # (bool (0=ok, 1=error), message)
+
+    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}"))
 
     # Check title
-    if title.endswith("..."):
-        errors.append("Title appears truncated")
-
-    if len(title) < 15:
-        errors.append("Title is too short")
-
-    if len(title) > 120:
-        errors.append("Title is too long")
-
-    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(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)
+    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")
 
-    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)
+    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)

.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

.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"

.github/workflows/pr-reviewed.yml

@@ -21,7 +21,7 @@ on:
     branches:
       - trunk
   pull_request:
-    types: [edited]
+    types: [opened, reopened, edited]
     branches:
       - trunk