[Concept,09/29] patman: Add review display and colour to series info

Message ID 20260501110040.1874719-10-sjg@u-boot.org
State New
Headers
Series patman: Review-flow improvements and shared helpers |

Commit Message

Simon Glass May 1, 2026, 11 a.m. UTC
  From: Simon Glass <sjg@chromium.org>

Reviews collected by 'patman review' land in the database but there is
no way to read them back from the command line - 'series info' shows
progress and tags but skips the review text entirely. Anyone wanting
to revisit a review has to dig through the patchwork web interface
or query the database by hand.

Extend 'series info' with a '-r' flag that prints the review text
for each patch alongside the existing summary. The flag accepts an
optional list of patch numbers (e.g. '-r 1 3') so a single patch can
be inspected without scrolling past the rest of the series. The output
is piped through the pager so long reviews scroll cleanly when stdout
is a terminal.

To make the dump readable at a glance, label fields are shown in blue,
version headers in yellow, accepted and approved markers in green,
quoted diff lines in magenta, review comments in white and notes in
cyan.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/patman/cmdline.py |   4 +-
 tools/patman/control.py |   3 +-
 tools/patman/cseries.py | 113 ++++++++++++++++++++++++++++++----------
 3 files changed, 90 insertions(+), 30 deletions(-)
  

Patch

diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py
index cbb6e4742b7..13aa63b76c8 100644
--- a/tools/patman/cmdline.py
+++ b/tools/patman/cmdline.py
@@ -294,7 +294,9 @@  def add_series_subparser(subparsers):
 
     series_subparsers.add_parser('get-link')
     series_subparsers.add_parser('inc')
-    series_subparsers.add_parser('info')
+    info = series_subparsers.add_parser('info')
+    info.add_argument('-r', '--reviews', nargs='*', type=int, default=None,
+                      help='Show review text (optionally for specific patches)')
     ls = series_subparsers.add_parser('ls', aliases=['list'])
     _add_archived(ls)
 
diff --git a/tools/patman/control.py b/tools/patman/control.py
index 28d53ef60d2..ac3a27a3e0e 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -202,7 +202,8 @@  def do_series(args, test_db=None, pwork=None, cser=None):
         elif args.subcmd == 'dec':
             cser.decrement(args.series, args.dry_run)
         elif args.subcmd == 'info':
-            cser.show_info(args.series)
+            cser.show_info(args.series,
+                           show_reviews=getattr(args, 'reviews', None))
         elif args.subcmd == 'gather':
             cser.gather(pwork, args.series, args.version, args.show_comments,
                         args.show_cover_comments, args.gather_tags,
diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py
index 3af9ef19eab..74e86eeeb37 100644
--- a/tools/patman/cseries.py
+++ b/tools/patman/cseries.py
@@ -13,6 +13,7 @@  import pygit2
 from u_boot_pylib import cros_subprocess
 from u_boot_pylib import gitutil
 from u_boot_pylib import terminal
+from u_boot_pylib.terminal import tprint
 from u_boot_pylib import tools
 from u_boot_pylib import tout
 
@@ -892,24 +893,42 @@  class Cseries(cser_helper.CseriesHelper):
             tout.notice(f"No review notes for '{ser.name}'")
             return
         for version, notes in all_notes:
-            terminal.tprint(f'\n--- v{version} ---',
+            tprint(f'\n--- v{version} ---',
                             colour=terminal.Color.YELLOW)
             print(notes)
             print()
 
-    def show_info(self, series):
+    def show_info(self, series, show_reviews=None):
         """Show detailed information about a series and all its versions
 
         Args:
             series (str): Series name, or None for current branch
+            show_reviews (list of int or None): If not None, show review
+                text. An empty list means all patches; otherwise only
+                the listed patch numbers (1-based).
         """
         ser, _ = self._parse_series_and_version(series, None)
         if not ser.idnum:
             raise ValueError(f"Series '{ser.name}' not found in database")
 
-        print(f"Series: {ser.name}")
-        print(f"  Description: {ser.desc}")
-        print(f"  Upstream: {ser.upstream or '(none)'}")
+        with terminal.pager():
+            self._show_info(ser, show_reviews)
+
+    def _show_info(self, ser, show_reviews):
+        """Show series info (called within a pager context)
+
+        Args:
+            ser (Series): Series object with idnum set
+            show_reviews (list of int or None): Patch numbers to show
+                reviews for, or empty list for all, or None for none
+        """
+        col = self.col
+        tprint('Series: ', newline=False, colour=col.BLUE, col=col)
+        tprint(ser.name, colour=col.WHITE, col=col)
+        tprint('  Description: ', newline=False, colour=col.BLUE, col=col)
+        tprint(ser.desc, col=col)
+        tprint('  Upstream: ', newline=False, colour=col.BLUE, col=col)
+        tprint(ser.upstream or '(none)', col=col)
 
         versions = self.db.ser_ver_get_for_series(ser.idnum)
         if not isinstance(versions, list):
@@ -917,32 +936,70 @@  class Cseries(cser_helper.CseriesHelper):
 
         for sv in versions:
             link_str = sv.link or '(none)'
-            print(f"\n  Version {sv.version}:")
-            print(f"    Link: {link_str}")
-            print(f"    Description: {sv.desc or '(none)'}")
+            tprint(f'\n  Version {sv.version}:', colour=col.YELLOW, col=col)
+            tprint('    Link: ', newline=False, colour=col.BLUE, col=col)
+            tprint(link_str, col=col)
+            tprint('    Description: ', newline=False, colour=col.BLUE, col=col)
+            tprint(sv.desc or '(none)', col=col)
             if sv.name:
-                print(f"    Cover: {sv.name}")
+                tprint('    Cover: ', newline=False, colour=col.BLUE, col=col)
+                tprint(sv.name, col=col)
             if sv.archive_tag:
-                print(f"    Archive tag: {sv.archive_tag}")
+                tprint('    Archive tag: ', newline=False, colour=col.BLUE,
+                       col=col)
+                tprint(sv.archive_tag, col=col)
 
-            # Show patches
-            try:
-                pclist = self.db.pcommit_get_list(sv.idnum)
-                print(f"    Patches: {len(pclist)}")
-                for pc in pclist:
-                    state = f' [{pc.state}]' if pc.state else ''
-                    print(f"      {pc.seq + 1}: {pc.subject}{state}")
-            except (ValueError, AttributeError):
-                pass
-
-            # Show notes if any
-            if sv.notes:
-                lines = sv.notes.strip().splitlines()
-                print(f"    Notes: {lines[0]}")
-                for line in lines[1:3]:
-                    print(f"           {line}")
-                if len(lines) > 3:
-                    print(f"           ... ({len(lines)} lines)")
+            self._show_version_info(sv, show_reviews)
+
+    def _show_version_info(self, sv, show_reviews):
+        """Show patches, reviews and notes for one series version
+
+        Args:
+            sv (SerVer): Series-version record to display
+            show_reviews (list of int or None): Patch numbers to show
+                reviews for, or empty list for all, or None for none
+        """
+        col = self.col
+
+        # Show patches
+        pclist = self.db.pcommit_get_list(sv.idnum)
+        tprint('    Patches: ', newline=False, colour=col.BLUE, col=col)
+        tprint(str(len(pclist)), col=col)
+        for pc in pclist:
+            state = f' [{pc.state}]' if pc.state else ''
+            colour = col.GREEN if pc.state == 'accepted' else None
+            tprint(f'      {pc.seq + 1}: {pc.subject}{state}', colour=colour,
+                   col=col)
+
+        # Show reviews if requested
+        if show_reviews is not None:
+            reviews = self.db.review_get_for_version(sv.idnum)
+            if reviews:
+                shown = [r for r in reviews if not show_reviews
+                         or r.seq in show_reviews]
+                tprint('    Reviews: ', newline=False, colour=col.BLUE, col=col)
+                tprint(f'{len(shown)}/{len(reviews)}', col=col)
+                for rev in shown:
+                    colour = col.GREEN if rev.approved else col.YELLOW
+                    stat = 'approved' if rev.approved else 'comments'
+                    tprint(f'      Patch {rev.seq}: [{stat}]', colour=colour,
+                           col=col)
+                    for line in rev.body.splitlines():
+                        if line.startswith('> '):
+                            tprint(f'        {line}', colour=col.MAGENTA,
+                                   col=col)
+                        else:
+                            tprint(f'        {line}', colour=col.WHITE, col=col)
+
+        # Show notes if any
+        if sv.notes:
+            lines = sv.notes.strip().splitlines()
+            tprint('    Notes: ', newline=False, colour=col.BLUE, col=col)
+            tprint(lines[0], colour=col.CYAN, col=col)
+            for line in lines[1:3]:
+                tprint(f'           {line}', colour=col.CYAN, col=col)
+            if len(lines) > 3:
+                tprint(f'           ... ({len(lines)} lines)', col=col)
 
     def set_upstream(self, series, ups, dry_run=False):
         """Set the upstream for a series