[Concept,14/29] patman: Hide review series from default listing

Message ID 20260501110040.1874719-15-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>

Review series are named after the cover letter title, which collides
with the user's own series and clutters 'patman series ls'. Use the
branch name format '<upstream>-<link>-review' (e.g. 'us-498633-review')
instead, and filter review series from the default listing. Add '-r'
to 'patman series ls' to show only review series.

Also fix series_find_review_by_name() to search by description rather
than name, since the name is now a branch identifier. The description
stores the cover letter title (or first patch subject) so matching
across versions still works.

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

 tools/patman/cmdline.py      |  2 ++
 tools/patman/control.py      |  3 ++-
 tools/patman/cseries.py      |  7 +++++--
 tools/patman/database.py     | 39 ++++++++++++++++++++++++------------
 tools/patman/patchwork.py    |  1 +
 tools/patman/review.py       | 31 ++++++++++++++++++++++------
 tools/patman/test_cseries.py |  2 +-
 7 files changed, 62 insertions(+), 23 deletions(-)
  

Patch

diff --git a/tools/patman/cmdline.py b/tools/patman/cmdline.py
index 758d84c0397..4ec9df7acb4 100644
--- a/tools/patman/cmdline.py
+++ b/tools/patman/cmdline.py
@@ -299,6 +299,8 @@  def add_series_subparser(subparsers):
                       help='Show review text (optionally for specific patches)')
     ls = series_subparsers.add_parser('ls', aliases=['list'])
     _add_archived(ls)
+    ls.add_argument('-r', '--reviews', action='store_true',
+                    help='Show only review series')
 
     mar = series_subparsers.add_parser('mark')
     mar.add_argument('-m', '--allow-marked', action='store_true',
diff --git a/tools/patman/control.py b/tools/patman/control.py
index ac3a27a3e0e..14d74028ae9 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -218,7 +218,8 @@  def do_series(args, test_db=None, pwork=None, cser=None):
         elif args.subcmd == 'inc':
             cser.increment(args.series, args.dry_run)
         elif args.subcmd == 'ls':
-            cser.series_list(args.include_archived)
+            cser.series_list(args.include_archived,
+                             reviews_only=getattr(args, 'reviews', False))
         elif args.subcmd == 'open':
             cser.open(pwork, args.series, args.version)
         elif args.subcmd == 'mark':
diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py
index 5ffc2989dd5..893e1b4d212 100644
--- a/tools/patman/cseries.py
+++ b/tools/patman/cseries.py
@@ -463,7 +463,7 @@  class Cseries(cser_helper.CseriesHelper):
 
         return summary
 
-    def series_list(self, include_archived=False):
+    def series_list(self, include_archived=False, reviews_only=False):
         """List all series
 
         Lines all series along with their description, number of patches
@@ -471,8 +471,11 @@  class Cseries(cser_helper.CseriesHelper):
 
         Args:
             include_archived (bool): True to include archived series also
+            reviews_only (bool): True to show only review series
         """
-        sdict = self.db.series_get_dict(include_archived)
+        sdict = self.db.series_get_dict(include_archived,
+                                        include_reviews=reviews_only,
+                                        reviews_only=reviews_only)
         print(f"{'Name':15}  {'Description':40}  Accepted  Us  Versions")
         border = f"{'-' * 15}  {'-' * 40}  --------  --  {'-' * 15}"
         print(border)
diff --git a/tools/patman/database.py b/tools/patman/database.py
index 50b9ea0dca5..7f1771cbf11 100644
--- a/tools/patman/database.py
+++ b/tools/patman/database.py
@@ -369,20 +369,29 @@  class Database:  # pylint:disable=R0904
         """
         return self.cur.rowcount
 
-    def _get_series_list(self, include_archived):
+    def _get_series_list(self, include_archived, include_reviews=False,
+                         reviews_only=False):
         """Get a list of Series objects from the database
 
         Args:
             include_archived (bool): True to include archives series
+            include_reviews (bool): True to include review series
+            reviews_only (bool): True to show only review series
 
         Return:
             list of Series
         """
+        conditions = []
+        if not include_archived:
+            conditions.append('archived = 0')
+        if reviews_only:
+            conditions.append("source = 'review'")
+        elif not include_reviews:
+            conditions.append("(source IS NULL OR source != 'review')")
+        where = ('WHERE ' + ' AND '.join(conditions)) if conditions else ''
         res = self.execute(
-            'SELECT id, name, desc, upstream FROM series ' +
-            ('WHERE archived = 0' if not include_archived else ''))
-        return [Series.from_fields(idnum=idnum, name=name, desc=desc,
-                                   ups=ups)
+            f'SELECT id, name, desc, upstream FROM series {where}')
+        return [Series.from_fields(idnum=idnum, name=name, desc=desc, ups=ups)
                 for idnum, name, desc, ups in res.fetchall()]
 
     # series functions
@@ -447,11 +456,14 @@  class Database:  # pylint:disable=R0904
             raise ValueError(f'No series found (id {idnum} len {len(recs)})')
         return recs[0]
 
-    def series_get_dict(self, include_archived=False):
+    def series_get_dict(self, include_archived=False,
+                        include_reviews=False, reviews_only=False):
         """Get a dict of Series objects from the database
 
         Args:
             include_archived (bool): True to include archives series
+            include_reviews (bool): True to include review series
+            reviews_only (bool): True to show only review series
 
         Return:
             OrderedDict:
@@ -459,7 +471,8 @@  class Database:  # pylint:disable=R0904
                 value: Series with idnum, name and desc filled out
         """
         sdict = OrderedDict()
-        for ser in self._get_series_list(include_archived):
+        for ser in self._get_series_list(include_archived, include_reviews,
+                                         reviews_only):
             sdict[ser.name] = ser
         return sdict
 
@@ -1440,14 +1453,14 @@  class Database:  # pylint:disable=R0904
         return res.fetchone()
 
     def series_find_review_by_name(self, name):
-        """Find a review series by its name
+        """Find a review series by its description
 
-        Looks for series with source='review' matching the given name,
-        so that new versions of a previously reviewed series can be
-        added under the same series record.
+        Looks for series with source='review' whose description matches
+        the given name, so that new versions of a previously reviewed
+        series can be added under the same series record.
 
         Args:
-            name (str): Series name to search for
+            name (str): Series description (cover letter title)
 
         Return:
             tuple or None: (series_id, name, max_version) if found
@@ -1456,6 +1469,6 @@  class Database:  # pylint:disable=R0904
             'SELECT s.id, s.name, MAX(sv.version) '
             'FROM series s '
             'JOIN ser_ver sv ON sv.series_id = s.id '
-            "WHERE s.source = 'review' AND s.name = ? "
+            "WHERE s.source = 'review' AND s.desc = ? "
             'GROUP BY s.id', (name,))
         return res.fetchone()
diff --git a/tools/patman/patchwork.py b/tools/patman/patchwork.py
index f71214d4b8f..de211f95878 100644
--- a/tools/patman/patchwork.py
+++ b/tools/patman/patchwork.py
@@ -180,6 +180,7 @@  class Patchwork:
         self.fake_request = None
         self.proj_id = None
         self.link_name = None
+        self.upstream = None
         self._show_progress = show_progress
         self.semaphore = asyncio.Semaphore(
             1 if single_thread else MAX_CONCURRENT)
diff --git a/tools/patman/review.py b/tools/patman/review.py
index 775ffc22a34..a850ec180df 100644
--- a/tools/patman/review.py
+++ b/tools/patman/review.py
@@ -1439,7 +1439,22 @@  def _clean_series_name(name):
     return name
 
 
-def _register_series(cser, clean_name, version, link, series_data):
+def _make_review_name(link, upstream=None):
+    """Build a series name for a review branch
+
+    Args:
+        link (str): Patchwork series link/ID
+        upstream (str or None): Upstream name
+
+    Returns:
+        str: Branch-style name, e.g. 'us-498633-review'
+    """
+    ups = upstream or 'pw'
+    return f'{ups}-{link}-review'
+
+
+def _register_series(cser, clean_name, version, link, series_data,
+                     upstream=None):
     """Register a series in the database for review
 
     Creates or finds the series record, adds a ser_ver entry and pcommit
@@ -1451,6 +1466,7 @@  def _register_series(cser, clean_name, version, link, series_data):
         version (int): Series version number
         link (str): Patchwork series link/ID
         series_data (dict): Series data from patchwork
+        upstream (str or None): Upstream name for branch naming
 
     Returns:
         tuple or None: (series_id, svid) or None if already reviewed
@@ -1465,12 +1481,13 @@  def _register_series(cser, clean_name, version, link, series_data):
         tout.notice(f"Previously reviewed '{db_name}' v{prev_version};"
                     f" adding v{version}")
     else:
+        branch_name = _make_review_name(link, upstream)
         series_id = cser.db.series_find_by_name(
-            clean_name, include_archived=True)
+            branch_name, include_archived=True)
         if not series_id:
             desc = series_data.get('cover_letter', {})
-            desc = desc.get('name', '') if desc else ''
-            series_id = cser.db.series_add(clean_name, desc)
+            desc = desc.get('name', '') if desc else clean_name
+            series_id = cser.db.series_add(branch_name, desc, ups=upstream)
         cser.db.series_set_source(series_id, 'review')
 
     svid = cser.db.ser_ver_add(series_id, version, link=str(link))
@@ -1604,7 +1621,8 @@  def _apply_and_check(ctx, link):
     Returns:
         str or None: Branch name, or None on failure
     """
-    branch_name = f'review{ctx.series_id}'
+    ups = ctx.pwork.upstream if ctx.pwork else None
+    branch_name = _make_review_name(link, ups)
     repo_path = gitutil.get_top_level()
     success, branch_name = apply_series_sync(ctx.pwork, link, branch_name,
         ctx.upstream_branch, repo_path)
@@ -1703,8 +1721,9 @@  def _find_or_register(ctx, args, clean_name, link):
         tuple or None: (series_id, svid) or None if already reviewed and
             not forcing
     """
+    ups = ctx.pwork.upstream if ctx.pwork else None
     result = _register_series(ctx.cser, clean_name, ctx.version, link,
-                              ctx.series_data)
+                              ctx.series_data, upstream=ups)
     if result is not None:
         return result
 
diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py
index 2d56b409a57..125e6db7b69 100644
--- a/tools/patman/test_cseries.py
+++ b/tools/patman/test_cseries.py
@@ -4616,7 +4616,7 @@  Date:   .*
         result = cser.db.series_find_by_link(str(self.REVIEW_LINK))
         self.assertIsNotNone(result)
         series_id, name, version, svid = result
-        self.assertEqual(self.REVIEW_NAME, name)
+        self.assertEqual(f'pw-{self.REVIEW_LINK}-review', name)
         self.assertEqual(1, version)
 
         # Check source is 'review'