[Concept,7/7] patman: Show a notice for each series subcommand

Message ID 20260214032632.3957279-8-sjg@u-boot.org
State New
Headers
Series patman: Improve series subcommand output |

Commit Message

Simon Glass Feb. 14, 2026, 3:26 a.m. UTC
  From: Simon Glass <simon.glass@canonical.com>

Several series subcommands complete silently or only log at the info
level, which is not visible with the default NOTICE log level. Add a
tout.notice() summary to each action subcommand so the user sees
confirmation of what was done.

For subcommands that already have a summary at info level, promote it
to notice. For those with no summary at all, add one. Also update
tests to suppress the new output where needed.

Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 tools/patman/cseries.py      | 40 ++++++++++++-----
 tools/patman/test_cseries.py | 85 +++++++++++++++++++++++-------------
 2 files changed, 83 insertions(+), 42 deletions(-)
  

Patch

diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py
index 937af58ba3e..c90020b7eb6 100644
--- a/tools/patman/cseries.py
+++ b/tools/patman/cseries.py
@@ -142,7 +142,7 @@  class Cseries(cser_helper.CseriesHelper):
             del_branch = repo.lookup_branch(del_name)
             branch_oid = del_branch.peel(pygit2.enums.ObjectType.COMMIT).oid
             del_branch.delete()
-            print(f"Deleted branch '{del_name}' {oid(branch_oid)}")
+            tout.info(f"Deleted branch '{del_name}' {oid(branch_oid)}")
 
         self.db.ser_ver_remove(ser.idnum, max_vers)
         if not dry_run:
@@ -150,6 +150,8 @@  class Cseries(cser_helper.CseriesHelper):
         else:
             self.rollback()
 
+        tout.notice(f"Decremented series '{ser.name}' to v{new_max}")
+
     def increment(self, series_name, dry_run=False):
         """Increment a series to the next version and create a new branch
 
@@ -191,7 +193,7 @@  class Cseries(cser_helper.CseriesHelper):
             self.rollback()
 
         # repo.head.set_target(amended)
-        tout.info(f'Added new branch {new_name}')
+        tout.notice(f"Incremented series '{ser.name}' to v{vers}")
         if dry_run:
             tout.info('Dry run completed')
 
@@ -372,7 +374,7 @@  class Cseries(cser_helper.CseriesHelper):
                 msg += f', {no_desc} missing description'
             if failed:
                 msg += f', {failed} updated failed'
-            tout.info(msg + f' ({requests} requests)')
+            tout.notice(msg + f' ({requests} requests)')
 
             tout.info('')
             tout.info(f"{'Name':15}  Version  {'Description':40}  Result")
@@ -466,6 +468,9 @@  class Cseries(cser_helper.CseriesHelper):
                     f'Marked commits {len(bad)}/{len(ser.commits)}')
         new_oid = self._mark_series(in_name, ser, dry_run=dry_run)
 
+        count = len(ser.commits)
+        tout.notice(f"Marked {count} commit{self.plural(count)}"
+                    f" in series '{name}'")
         if dry_run:
             tout.info('Dry run completed')
         return new_oid
@@ -510,6 +515,9 @@  class Cseries(cser_helper.CseriesHelper):
             else:
                 vals.info = 'no mark'
 
+        count = len(ser.commits)
+        tout.notice(f"Unmarked {count} commit{self.plural(count)}"
+                    f" in series '{name}'")
         if dry_run:
             tout.info('Dry run completed')
         return vals.oid
@@ -756,7 +764,7 @@  class Cseries(cser_helper.CseriesHelper):
         else:
             self.rollback()
 
-        tout.info(f"Renamed series '{series}' to '{name}'")
+        tout.notice(f"Renamed series '{series}' to '{name}'")
         if dry_run:
             tout.info('Dry run completed')
 
@@ -916,6 +924,9 @@  class Cseries(cser_helper.CseriesHelper):
 
         self.db.series_set_archived(ser.idnum, True)
         self.commit()
+        count = len(tag_info)
+        tout.notice(f"Archived series '{ser.name}'"
+                    f" ({count} version{self.plural(count)})")
 
     def unarchive(self, series):
         """Unarchive a series
@@ -958,6 +969,9 @@  class Cseries(cser_helper.CseriesHelper):
             self.db.ser_ver_set_archive_tag(idnum, None)
 
         self.commit()
+        count = len(tag_info)
+        tout.notice(f"Unarchived series '{ser.name}'"
+                    f" ({count} version{self.plural(count)})")
 
     def status(self, pwork, series, version, show_comments,
                show_cover_comments=False):
@@ -1029,9 +1043,9 @@  class Cseries(cser_helper.CseriesHelper):
             updated, updated_cover = self._sync_one(
                 svid, ser.name, version, show_comments, show_cover_comments,
                 gather_tags, cover, patches, dry_run)
-            tout.info(f"{updated} patch{'es' if updated != 1 else ''}"
-                      f"{' and cover letter' if updated_cover else ''} "
-                      f'updated ({stats.request_count} requests)')
+            tout.notice(f"{updated} patch{'es' if updated != 1 else ''}"
+                        f"{' and cover letter' if updated_cover else ''} "
+                        f'updated ({stats.request_count} requests)')
 
             if not dry_run:
                 self.commit()
@@ -1064,7 +1078,7 @@  class Cseries(cser_helper.CseriesHelper):
                 add_newline = gather_tags
 
             tout.info('')
-            tout.info(
+            tout.notice(
                 f"{tot_updated} patch{'es' if tot_updated != 1 else ''} and "
                 f"{tot_cover} cover letter{'s' if tot_cover != 1 else ''} "
                 f'updated, {missing} missing '
@@ -1084,6 +1098,7 @@  class Cseries(cser_helper.CseriesHelper):
         """
         self.db.upstream_add(name, url)
         self.commit()
+        tout.notice(f"Added upstream '{name}' ({url})")
 
     def upstream_list(self):
         """List the upstream repos
@@ -1106,6 +1121,8 @@  class Cseries(cser_helper.CseriesHelper):
         """
         self.db.upstream_set_default(name)
         self.commit()
+        if name:
+            tout.notice(f"Set default upstream to '{name}'")
 
     def upstream_get_default(self):
         """Get the default upstream target
@@ -1123,6 +1140,7 @@  class Cseries(cser_helper.CseriesHelper):
         """
         self.db.upstream_delete(name)
         self.commit()
+        tout.notice(f"Deleted upstream '{name}'")
 
     def version_remove(self, name, version, dry_run=False):
         """Remove a version of a series from the database
@@ -1147,7 +1165,7 @@  class Cseries(cser_helper.CseriesHelper):
         else:
             self.rollback()
 
-        tout.info(f"Removed version {version} from series '{name}'")
+        tout.notice(f"Removed version {version} from series '{name}'")
         if dry_run:
             tout.info('Dry run completed')
 
@@ -1194,7 +1212,7 @@  class Cseries(cser_helper.CseriesHelper):
         else:
             self.rollback()
 
-        tout.info(f"Changed version {version} in series '{ser.name}' "
-                  f"to {new_version} named '{new_name}'")
+        tout.notice(f"Changed version {version} in series '{ser.name}' "
+                    f"to {new_version} named '{new_name}'")
         if dry_run:
             tout.info('Dry run completed')
diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py
index 3aa8e0e739e..c8af0bb1db7 100644
--- a/tools/patman/test_cseries.py
+++ b/tools/patman/test_cseries.py
@@ -772,7 +772,8 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
     def test_series_list_archived(self):
         """Archive a series and test listing it"""
         self.setup_second()
-        self.cser.archive('first')
+        with terminal.capture():
+            self.cser.archive('first')
         with terminal.capture() as (out, _):
             self.run_args('series', 'ls', pwork=True)
         lines = out.getvalue().splitlines()
@@ -877,7 +878,7 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
                          f'- add v2: {HASH_RE} as {HASH_RE} spi: SPI fixes')
         self.assertRegex(
             next(itr), f'Updating branch first2 from {HASH_RE} to {HASH_RE}')
-        self.assertEqual('Added new branch first2', next(itr))
+        self.assertEqual("Incremented series 'first' to v2", next(itr))
         return itr
 
     def test_series_link(self):
@@ -1532,9 +1533,11 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
         cser = next(cor)
 
         # Archive it and make sure it is invisible
-        cser.archive('first')
+        with terminal.capture():
+            cser.archive('first')
         cser = next(cor)
-        cser.unarchive('first')
+        with terminal.capture():
+            cser.unarchive('first')
         self.assertFalse(next(cor))
         cor.close()
 
@@ -1544,11 +1547,13 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
         cser = next(cor)
 
         # Archive it and make sure it is invisible
-        self.run_args('series', '-s', 'first', 'archive', pwork=True,
-                      cser=cser)
+        with terminal.capture():
+            self.run_args('series', '-s', 'first', 'archive', pwork=True,
+                          cser=cser)
         next(cor)
-        self.run_args('series', '-s', 'first', 'unarchive', pwork=True,
-                      cser=cser)
+        with terminal.capture():
+            self.run_args('series', '-s', 'first', 'unarchive', pwork=True,
+                          cser=cser)
         self.assertFalse(next(cor))
         cor.close()
 
@@ -1696,10 +1701,11 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
         with terminal.capture() as (out, _):
             cser.decrement('first')
         lines = out.getvalue().splitlines()
-        self.assertEqual(2, len(lines))
+        self.assertEqual(3, len(lines))
         self.assertEqual("Removing series 'first' v2", lines[0])
         self.assertEqual(
             f"Deleted branch 'first2' {str(branch_oid)[:10]}", lines[1])
+        self.assertEqual("Decremented series 'first' to v1", lines[2])
 
         svdict = cser.get_ser_ver_dict()
         self.assertEqual(1, len(svdict))
@@ -1720,12 +1726,14 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
         """Test adding an upsream"""
         cser = self.get_cser()
 
-        cser.upstream_add('us', 'https://one')
+        with terminal.capture():
+            cser.upstream_add('us', 'https://one')
         ulist = cser.get_upstream_dict()
         self.assertEqual(1, len(ulist))
         self.assertEqual(('https://one', None), ulist['us'])
 
-        cser.upstream_add('ci', 'git@two')
+        with terminal.capture():
+            cser.upstream_add('ci', 'git@two')
         ulist = cser.get_upstream_dict()
         self.assertEqual(2, len(ulist))
         self.assertEqual(('https://one', None), ulist['us'])
@@ -1762,17 +1770,21 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
             cser.upstream_set_default('us')
         self.assertEqual("No such upstream 'us'", str(exc.exception))
 
-        cser.upstream_add('us', 'https://one')
-        cser.upstream_add('ci', 'git@two')
+        with terminal.capture():
+            cser.upstream_add('us', 'https://one')
+            cser.upstream_add('ci', 'git@two')
 
         self.assertIsNone(cser.upstream_get_default())
 
-        cser.upstream_set_default('us')
+        with terminal.capture():
+            cser.upstream_set_default('us')
         self.assertEqual('us', cser.upstream_get_default())
 
-        cser.upstream_set_default('us')
+        with terminal.capture():
+            cser.upstream_set_default('us')
 
-        cser.upstream_set_default('ci')
+        with terminal.capture():
+            cser.upstream_set_default('ci')
         self.assertEqual('ci', cser.upstream_get_default())
 
         with terminal.capture() as (out, _):
@@ -1792,19 +1804,22 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
         self.assertEqual("patman: ValueError: No such upstream 'us'",
                          out.getvalue().strip().splitlines()[-1])
 
-        self.run_args('upstream', 'add', 'us', 'https://one')
-        self.run_args('upstream', 'add', 'ci', 'git@two')
+        with terminal.capture():
+            self.run_args('upstream', 'add', 'us', 'https://one')
+            self.run_args('upstream', 'add', 'ci', 'git@two')
 
         with terminal.capture() as (out, _):
             self.run_args('upstream', 'default')
         self.assertEqual('unset', out.getvalue().strip())
 
-        self.run_args('upstream', 'default', 'us')
+        with terminal.capture():
+            self.run_args('upstream', 'default', 'us')
         with terminal.capture() as (out, _):
             self.run_args('upstream', 'default')
         self.assertEqual('us', out.getvalue().strip())
 
-        self.run_args('upstream', 'default', 'ci')
+        with terminal.capture():
+            self.run_args('upstream', 'default', 'ci')
         with terminal.capture() as (out, _):
             self.run_args('upstream', 'default')
         self.assertEqual('ci', out.getvalue().strip())
@@ -1825,14 +1840,17 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
             cser.upstream_delete('us')
         self.assertEqual("No such upstream 'us'", str(exc.exception))
 
-        cser.upstream_add('us', 'https://one')
-        cser.upstream_add('ci', 'git@two')
+        with terminal.capture():
+            cser.upstream_add('us', 'https://one')
+            cser.upstream_add('ci', 'git@two')
 
-        cser.upstream_set_default('us')
-        cser.upstream_delete('us')
+        with terminal.capture():
+            cser.upstream_set_default('us')
+            cser.upstream_delete('us')
         self.assertIsNone(cser.upstream_get_default())
 
-        cser.upstream_delete('ci')
+        with terminal.capture():
+            cser.upstream_delete('ci')
         ulist = cser.get_upstream_dict()
         self.assertFalse(ulist)
 
@@ -1843,17 +1861,20 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
         self.assertEqual("patman: ValueError: No such upstream 'us'",
                          out.getvalue().strip().splitlines()[-1])
 
-        self.run_args('us', 'add', 'us', 'https://one')
-        self.run_args('us', 'add', 'ci', 'git@two')
+        with terminal.capture():
+            self.run_args('us', 'add', 'us', 'https://one')
+            self.run_args('us', 'add', 'ci', 'git@two')
 
-        self.run_args('upstream', 'default', 'us')
-        self.run_args('upstream', 'delete', 'us')
+        with terminal.capture():
+            self.run_args('upstream', 'default', 'us')
+            self.run_args('upstream', 'delete', 'us')
         with terminal.capture() as (out, _):
             self.run_args('upstream', 'default', 'us', expect_ret=1)
         self.assertEqual("patman: ValueError: No such upstream 'us'",
                          out.getvalue().strip())
 
-        self.run_args('upstream', 'delete', 'ci')
+        with terminal.capture():
+            self.run_args('upstream', 'delete', 'ci')
         with terminal.capture() as (out, _):
             self.run_args('upstream', 'list')
         self.assertFalse(out.getvalue().strip())
@@ -2003,6 +2024,7 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
             f'- unmarked: {HASH_RE} as {HASH_RE} spi: SPI fixes')
         self.assertRegex(
             next(itr), f'Updating branch first from {HASH_RE} to {HASH_RE}')
+        self.assertEqual("Unmarked 2 commits in series 'first'", next(itr))
         self.assertEqual('Dry run completed', next(itr))
 
         with self.stage('unmark'):
@@ -3086,7 +3108,8 @@  Date:   .*
     def test_series_progress_all_archived(self):
         """Test showing progress for all cseries including archived ones"""
         self.setup_second()
-        self.cser.archive('first')
+        with terminal.capture():
+            self.cser.archive('first')
 
         with self.stage('progress without archived'):
             with terminal.capture() as (out, _):