From patchwork Fri May 1 11:00:04 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 2255 Return-Path: X-Original-To: u-boot-concept@u-boot.org Delivered-To: u-boot-concept@u-boot.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1777633298; bh=aeGj/dUrGk+A3dZEGxKwmZggRmUMb7GrGhZzVBUzMUk=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=NPzpq7Qx81eMsgDmqmHzGjFOggK8DXZX4beKtdD0SpU8Sxl87VjTTD6Ocj846DZrr opNvrhIJ/pNSY76z90bV+sF0XAsAV9awT6xRevmlii9cH0PjPssNVhvog/3xQVryXF IxWbze6iWEPw/+exRSSu0Vc23Tj/H55HCt5eT9zA= Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 3DBDF6A6F8 for ; Fri, 1 May 2026 05:01:38 -0600 (MDT) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id b1P4v_MLfdE8 for ; Fri, 1 May 2026 05:01:38 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1777633298; bh=aeGj/dUrGk+A3dZEGxKwmZggRmUMb7GrGhZzVBUzMUk=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=NPzpq7Qx81eMsgDmqmHzGjFOggK8DXZX4beKtdD0SpU8Sxl87VjTTD6Ocj846DZrr opNvrhIJ/pNSY76z90bV+sF0XAsAV9awT6xRevmlii9cH0PjPssNVhvog/3xQVryXF IxWbze6iWEPw/+exRSSu0Vc23Tj/H55HCt5eT9zA= Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 2C8E26A7AF for ; Fri, 1 May 2026 05:01:38 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1777633297; bh=V6XjCj9B3atOTYRXTHz9A7iLQT5L1lfMkVFAagC/hE0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ciYd+ythgELSc5ydIT0cL+moSzDg96vw1KScE6mU4q2CW2+P4fdty9iG80fZAJtA/ hk9MxAVA3c+2vPqZOt+dHA0V5MIYMwTtybJOhf+AN6BgnTLjlxDKrXqIKS0NWMTmxM VepytaIm6Bs2+Do1JpDwproGHXtoyM3pe1aU5VRs= Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 0BFB56A6F8; Fri, 1 May 2026 05:01:37 -0600 (MDT) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10026) with ESMTP id mpo2GQPaAdPM; Fri, 1 May 2026 05:01:36 -0600 (MDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1777633292; bh=Yb3T5/CDBHsfLiOWw37NLWF0FwpcSDh0ZeG7YCiTQos=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=biri/GsYHXAky6RsYjqNVXsC8uo1YbwI4dtZYDyvZ44b+OC/n1dtZRgdw6dHMxln2 8TTKtIXWZL6tmk9SVhRCIFc4XNIuam9zeA6fjZ/D69Vur5ICcT/NCRCDyhIPUUGHwr Ie5SHzh//k1hRH/78FRvFyDMBxAD4FGLC/Nbya90= Received: from u-boot.org (unknown [174.51.25.52]) by mail.u-boot.org (Postfix) with ESMTPSA id 9E7A56A7AF; Fri, 1 May 2026 05:01:32 -0600 (MDT) From: Simon Glass To: U-Boot Concept Date: Fri, 1 May 2026 05:00:04 -0600 Message-ID: <20260501110040.1874719-13-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20260501110040.1874719-1-sjg@u-boot.org> References: <20260501110040.1874719-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: S7O7PJJTGTUPCKABRETPP3B6HBVVFV4F X-Message-ID-Hash: S7O7PJJTGTUPCKABRETPP3B6HBVVFV4F X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Simon Glass X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 12/29] patman: Refine review prompts and fix review handling List-Id: Discussion and patches related to U-Boot Concept Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: Simon Glass The AI review output mixes double and single quotes around identifiers and lets periods land directly after code tokens, both of which make quoted function names inconvenient to copy from a terminal. Update the per-patch and cover-letter prompts to use single quotes around non-string identifiers and to avoid trailing periods after code references. Reviews with comments drop the diffstat that approved reviews carry, so a recipient cannot tell at a glance which files the comments cover. Quote up to 30 lines of diffstat after the commit message in those reviews, matching the format already produced for approved ones. If a review is interrupted after the series row is registered but before any review rows are stored, the next attempt silently exits instead of resuming. The '--force' path also re-derives lookup data that the resume path already has. Fix '_find_or_register()' to resume from a half-registered series and rework the '--force' path to reuse the existing IDs. Signed-off-by: Simon Glass --- tools/patman/review.py | 45 ++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 15 deletions(-) -- 2.43.0 diff --git a/tools/patman/review.py b/tools/patman/review.py index eb2e46a0431..775ffc22a34 100644 --- a/tools/patman/review.py +++ b/tools/patman/review.py @@ -379,8 +379,11 @@ Rules: - NEVER use backticks — this is plain-text email, not markdown. For functions, always use parentheses with no quotes: malloc() not 'malloc()' or `malloc`. For other identifiers (variables, struct - names, filenames) do not quote them unless they are common English - words where the reader might not realise they are code references + names, filenames) use single quotes: 'my_var' not "my_var", unless + it is a string literal (use double quotes for strings). Do not + quote identifiers that are obviously code (e.g. CONFIG_FOO) +- Never put a period directly after a code identifier — rephrase, + omit the period, or use an em dash to start the next clause - If another reviewer has already made a point, do NOT repeat it, rephrase it, or add to it — skip it entirely - Focus on what is wrong and what to do instead @@ -479,8 +482,12 @@ VERDICT: changes_needed Rules: - NEVER use backticks — this is plain-text email, not markdown. For functions, always use parentheses with no quotes: malloc() not - 'malloc()' or `malloc`. For other identifiers do not quote them - unless they are common English words that might confuse the reader + 'malloc()' or `malloc`. For other identifiers (variables, struct + names, filenames) use single quotes: 'my_var' not "my_var", unless + it is a string literal (use double quotes for strings). Do not + quote identifiers that are obviously code (e.g. CONFIG_FOO) +- Never put a period directly after a code identifier — rephrase, + omit the period, or use an em dash to start the next clause - Use {ctx.spelling} spelling - Be brief — only raise series-level concerns, not per-patch nits - Do NOT repeat issues that belong on individual patches @@ -657,7 +664,14 @@ def _format_with_comments(ctx, greeting, verdict, comments, lines.append(f'> {cl}') if len(msg_lines) > max_quote: lines.append('> [...]') - lines.append('') + diffstat = getattr(ctx, 'diffstat', None) + if diffstat: + ds_lines = diffstat.strip().splitlines() + if len(ds_lines) <= 30: + lines.append('>') + for dl in ds_lines: + lines.append(f'> {dl}') + lines.append('') for hunk, comment in comments: if hunk: @@ -1698,10 +1712,15 @@ def _find_or_register(ctx, args, clean_name, link): if not existing: return None - _, _, _, svid = existing + series_id, _, _, svid = existing reviews = ctx.cser.db.review_get_for_version(svid) - if reviews and not args.force: + if not reviews: + # Interrupted previous attempt — resume with existing record + tout.notice('Resuming incomplete review') + return series_id, svid + + if not args.force: _, db_name, db_version, _ = existing tout.notice(f"Already reviewed: '{db_name}' v{db_version}") _show_reviews(reviews, ctx.series_data) @@ -1710,14 +1729,10 @@ def _find_or_register(ctx, args, clean_name, link): ctx.cser) return None - if reviews and args.force: - ctx.cser.db.review_delete_for_version(svid) - ctx.cser.commit() - tout.notice('Re-reviewing (forced)') - - # Re-register for re-review - return _register_series(ctx.cser, clean_name, ctx.version, link, - ctx.series_data) + ctx.cser.db.review_delete_for_version(svid) + ctx.cser.commit() + tout.notice('Re-reviewing (forced)') + return series_id, svid def do_review(args, pwork, cser):