Patchwork [U-Boot,8/8] patman: Add Series-process-log tag to sort/uniq change logs

login
register
mail settings
Submitter Simon Glass
Date March 21, 2013, 2:43 a.m.
Message ID <1363833781-14557-9-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/229534/
State Superseded, archived
Delegated to: Simon Glass
Headers show

Comments

Simon Glass - March 21, 2013, 2:43 a.m.
For some series with lots of changes it is annoying that duplicate change
log items are not caught. It is also helpful sometimes to sort the change
logs.

Add a Series-process-log tag to enable this, which can be placed in a
commit to control this.

The change to the Cc: line is to fix a checkpatch warning.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/patman/README         | 8 +++++++-
 tools/patman/patchstream.py | 2 +-
 tools/patman/series.py      | 8 ++++++--
 3 files changed, 14 insertions(+), 4 deletions(-)
Doug Anderson - March 21, 2013, 5:51 p.m.
Simon,

On Wed, Mar 20, 2013 at 7:43 PM, Simon Glass <sjg@chromium.org> wrote:
> For some series with lots of changes it is annoying that duplicate change
> log items are not caught. It is also helpful sometimes to sort the change
> logs.
>
> Add a Series-process-log tag to enable this, which can be placed in a
> commit to control this.
>
> The change to the Cc: line is to fix a checkpatch warning.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  tools/patman/README         | 8 +++++++-
>  tools/patman/patchstream.py | 2 +-
>  tools/patman/series.py      | 8 ++++++--
>  3 files changed, 14 insertions(+), 4 deletions(-)

Not sure I'd find this terribly useful myself, but I don't see
anything wrong with it.  I think my change log items tend to be more
than one line long for one...

> +                if not ('uniq' in process_it and text in out):

optional: My brain had a hard time processing this.  I did the logic
transformation myself:

  if 'uniq' not in process_it or text not in out:


Also: Do you really want the "process_it" to be so free-form?  That
seems like it might be asking for disaster.  Why not specify that it's
comma-separated and be done.

-Doug
Simon Glass - March 26, 2013, 10:15 p.m.
Hi Doug,

On Thu, Mar 21, 2013 at 10:51 AM, Doug Anderson <dianders@chromium.org> wrote:
> Simon,
>
> On Wed, Mar 20, 2013 at 7:43 PM, Simon Glass <sjg@chromium.org> wrote:
>> For some series with lots of changes it is annoying that duplicate change
>> log items are not caught. It is also helpful sometimes to sort the change
>> logs.
>>
>> Add a Series-process-log tag to enable this, which can be placed in a
>> commit to control this.
>>
>> The change to the Cc: line is to fix a checkpatch warning.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  tools/patman/README         | 8 +++++++-
>>  tools/patman/patchstream.py | 2 +-
>>  tools/patman/series.py      | 8 ++++++--
>>  3 files changed, 14 insertions(+), 4 deletions(-)
>
> Not sure I'd find this terribly useful myself, but I don't see
> anything wrong with it.  I think my change log items tend to be more
> than one line long for one...

I use it a lot. Brevity is a virtue :-)

>
>> +                if not ('uniq' in process_it and text in out):
>
> optional: My brain had a hard time processing this.  I did the logic
> transformation myself:
>
>   if 'uniq' not in process_it or text not in out:

Will update.

>
>
> Also: Do you really want the "process_it" to be so free-form?  That
> seems like it might be asking for disaster.  Why not specify that it's
> comma-separated and be done.

OK I'll add processing to check that.

Regards,
Simon

Patch

diff --git a/tools/patman/README b/tools/patman/README
index d4e7f36..566f54d 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -225,9 +225,15 @@  Series-changes: n
 	to update the log there and then, knowing that the script will
 	do the rest.
 
-Cc: Their Name <email>
+ Cc: Their Name <email>
 	This copies a single patch to another email address.
 
+Series-process-log: sort, uniq
+	This tells patman to sort and/or uniq the change logs. It is
+	assumed that each change log entry is only a single line long.
+	Use 'sort' to sort the entries, and 'uniq' to include only
+	unique entries. If omitted, no change log processing is done.
+
 Various other tags are silently removed, like these Chrome OS and
 Gerrit tags:
 
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index a4f2f31..9d8a918 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -46,7 +46,7 @@  re_cover = re.compile('^Cover-letter:')
 re_cover_cc = re.compile('^Cover-letter-cc: *(.*)')
 
 # Patch series tag
-re_series = re.compile('^Series-(\w*): *(.*)')
+re_series = re.compile('^Series-([a-z-]*): *(.*)')
 
 # Commit tags that we want to collect and keep
 re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Cc): (.*)')
diff --git a/tools/patman/series.py b/tools/patman/series.py
index fe919be..7080a55 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -28,7 +28,7 @@  import terminal
 
 # Series-xxx tags that we understand
 valid_series = ['to', 'cc', 'version', 'changes', 'prefix', 'notes', 'name',
-                'cover-cc']
+                'cover-cc', 'process_log']
 
 class Series(dict):
     """Holds information about a patch series, including all tags.
@@ -167,15 +167,19 @@  class Series(dict):
             etc.
         """
         final = []
+        process_it = self.get('process_log', '')
         need_blank = False
         for change in sorted(self.changes, reverse=True):
             out = []
             for this_commit, text in self.changes[change]:
                 if commit and this_commit != commit:
                     continue
-                out.append(text)
+                if not ('uniq' in process_it and text in out):
+                    out.append(text)
             line = 'Changes in v%d:' % change
             have_changes = len(out) > 0
+            if 'sort' in process_it:
+                out = sorted(out)
             if have_changes:
                 out.insert(0, line)
             else: