diff mbox series

[U-Boot] patman: add option for limiting the Cc list

Message ID 20180526231027.18621-1-judge.packham@gmail.com
State Superseded
Headers show
Series [U-Boot] patman: add option for limiting the Cc list | expand

Commit Message

Chris Packham May 26, 2018, 11:10 p.m. UTC
Many mailing-lists consider a long Cc list a sign of spam and will
either drop the message or mark it for moderation. Because patman
automatically invokes get_maintainer.pl the Cc list can expand
unexpectedly. Allow the user to specify a limit for the Cc list.

This limit is applied after removing any known bouncing addresses. By
default no limit is applied.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
I've fallen foul of the u-boot ML Cc limit a few times recently. I'm not
sure what the actual limit is so I've left patman's default behaviour
unlimited.

 tools/patman/patman.py | 4 +++-
 tools/patman/series.py | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Simon Glass May 27, 2018, 12:53 a.m. UTC | #1
Hi Chris,

On 26 May 2018 at 17:10, Chris Packham <judge.packham@gmail.com> wrote:
> Many mailing-lists consider a long Cc list a sign of spam and will
> either drop the message or mark it for moderation. Because patman
> automatically invokes get_maintainer.pl the Cc list can expand
> unexpectedly. Allow the user to specify a limit for the Cc list.
>
> This limit is applied after removing any known bouncing addresses. By
> default no limit is applied.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> I've fallen foul of the u-boot ML Cc limit a few times recently. I'm not
> sure what the actual limit is so I've left patman's default behaviour
> unlimited.
>
>  tools/patman/patman.py | 4 +++-
>  tools/patman/series.py | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/patman/patman.py b/tools/patman/patman.py
> index 8d2c78235a7e..7dd7a1c7b61a 100755
> --- a/tools/patman/patman.py
> +++ b/tools/patman/patman.py
> @@ -38,6 +38,8 @@ parser.add_option('-i', '--ignore-errors', action='store_true',
>  parser.add_option('-m', '--no-maintainers', action='store_false',
>         dest='add_maintainers', default=True,
>         help="Don't cc the file maintainers automatically")
> +parser.add_option('-l', '--limit-cc', dest='limit', type='int',
> +       default=0, help='Limit the cc list to LIMIT entries [default: %default]')

Could we have a default of None? If that works then I think it would
be preferable, so that 0 means no CC.

>  parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
>         default=False, help="Do a dry run (create but don't email patches)")
>  parser.add_option('-p', '--project', default=project.DetectProject(),
> @@ -157,7 +159,7 @@ else:
>
>      cc_file = series.MakeCcFile(options.process_tags, cover_fname,
>                                  not options.ignore_bad_tags,
> -                                options.add_maintainers)
> +                                options.add_maintainers, options.limit)
>
>      # Email the patches out (giving the user time to check / cancel)
>      cmd = ''
> diff --git a/tools/patman/series.py b/tools/patman/series.py
> index d526d4ee91d3..2066b97e73ff 100644
> --- a/tools/patman/series.py
> +++ b/tools/patman/series.py
> @@ -202,7 +202,7 @@ class Series(dict):
>              print(col.Color(col.RED, str))
>
>      def MakeCcFile(self, process_tags, cover_fname, raise_on_error,
> -                   add_maintainers):
> +                   add_maintainers, limit=0):

Please add an arg comment for limit

>          """Make a cc file for us to use for per-commit Cc automation
>
>          Also stores in self._generated_cc to make ShowActions() faster.
> @@ -238,6 +238,8 @@ class Series(dict):
>                  print(col.Color(col.YELLOW, 'Skipping "%s"' % x))
>              cc = set(cc) - set(settings.bounces)
>              cc = [m.encode('utf-8') if type(m) != str else m for m in cc]
> +            if limit > 0:
> +                cc = cc[:limit]
>              all_ccs += cc
>              print(commit.patch, ', '.join(set(cc)), file=fd)
>              self._generated_cc[commit.patch] = cc
> --
> 2.17.0
>

Regards,
Simon
Chris Packham May 27, 2018, 9:29 a.m. UTC | #2
On Sun, 27 May 2018, 12:53 PM Simon Glass, <sjg@chromium.org> wrote:

> Hi Chris,
>
> On 26 May 2018 at 17:10, Chris Packham <judge.packham@gmail.com> wrote:
> > Many mailing-lists consider a long Cc list a sign of spam and will
> > either drop the message or mark it for moderation. Because patman
> > automatically invokes get_maintainer.pl the Cc list can expand
> > unexpectedly. Allow the user to specify a limit for the Cc list.
> >
> > This limit is applied after removing any known bouncing addresses. By
> > default no limit is applied.
> >
> > Signed-off-by: Chris Packham <judge.packham@gmail.com>
> > ---
> > I've fallen foul of the u-boot ML Cc limit a few times recently. I'm not
> > sure what the actual limit is so I've left patman's default behaviour
> > unlimited.
> >
> >  tools/patman/patman.py | 4 +++-
> >  tools/patman/series.py | 4 +++-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/patman/patman.py b/tools/patman/patman.py
> > index 8d2c78235a7e..7dd7a1c7b61a 100755
> > --- a/tools/patman/patman.py
> > +++ b/tools/patman/patman.py
> > @@ -38,6 +38,8 @@ parser.add_option('-i', '--ignore-errors',
> action='store_true',
> >  parser.add_option('-m', '--no-maintainers', action='store_false',
> >         dest='add_maintainers', default=True,
> >         help="Don't cc the file maintainers automatically")
> > +parser.add_option('-l', '--limit-cc', dest='limit', type='int',
> > +       default=0, help='Limit the cc list to LIMIT entries [default:
> %default]')
>
> Could we have a default of None? If that works then I think it would
> be preferable, so that 0 means no CC.
>

That would make -m the same as -l 0. But regardless None is a better fit
for unset.


> >  parser.add_option('-n', '--dry-run', action='store_true',
> dest='dry_run',
> >         default=False, help="Do a dry run (create but don't email
> patches)")
> >  parser.add_option('-p', '--project', default=project.DetectProject(),
> > @@ -157,7 +159,7 @@ else:
> >
> >      cc_file = series.MakeCcFile(options.process_tags, cover_fname,
> >                                  not options.ignore_bad_tags,
> > -                                options.add_maintainers)
> > +                                options.add_maintainers, options.limit)
> >
> >      # Email the patches out (giving the user time to check / cancel)
> >      cmd = ''
> > diff --git a/tools/patman/series.py b/tools/patman/series.py
> > index d526d4ee91d3..2066b97e73ff 100644
> > --- a/tools/patman/series.py
> > +++ b/tools/patman/series.py
> > @@ -202,7 +202,7 @@ class Series(dict):
> >              print(col.Color(col.RED, str))
> >
> >      def MakeCcFile(self, process_tags, cover_fname, raise_on_error,
> > -                   add_maintainers):
> > +                   add_maintainers, limit=0):
>
> Please add an arg comment for limit
>

Will do. Also since there is only one caller of this which passes limit
should I make this non-optional.


> >          """Make a cc file for us to use for per-commit Cc automation
> >
> >          Also stores in self._generated_cc to make ShowActions() faster.
> > @@ -238,6 +238,8 @@ class Series(dict):
> >                  print(col.Color(col.YELLOW, 'Skipping "%s"' % x))
> >              cc = set(cc) - set(settings.bounces)
> >              cc = [m.encode('utf-8') if type(m) != str else m for m in
> cc]
> > +            if limit > 0:
> > +                cc = cc[:limit]
> >              all_ccs += cc
> >              print(commit.patch, ', '.join(set(cc)), file=fd)
> >              self._generated_cc[commit.patch] = cc
> > --
> > 2.17.0
> >
>
> Regards,
> Simon
>
diff mbox series

Patch

diff --git a/tools/patman/patman.py b/tools/patman/patman.py
index 8d2c78235a7e..7dd7a1c7b61a 100755
--- a/tools/patman/patman.py
+++ b/tools/patman/patman.py
@@ -38,6 +38,8 @@  parser.add_option('-i', '--ignore-errors', action='store_true',
 parser.add_option('-m', '--no-maintainers', action='store_false',
        dest='add_maintainers', default=True,
        help="Don't cc the file maintainers automatically")
+parser.add_option('-l', '--limit-cc', dest='limit', type='int',
+       default=0, help='Limit the cc list to LIMIT entries [default: %default]')
 parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
        default=False, help="Do a dry run (create but don't email patches)")
 parser.add_option('-p', '--project', default=project.DetectProject(),
@@ -157,7 +159,7 @@  else:
 
     cc_file = series.MakeCcFile(options.process_tags, cover_fname,
                                 not options.ignore_bad_tags,
-                                options.add_maintainers)
+                                options.add_maintainers, options.limit)
 
     # Email the patches out (giving the user time to check / cancel)
     cmd = ''
diff --git a/tools/patman/series.py b/tools/patman/series.py
index d526d4ee91d3..2066b97e73ff 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -202,7 +202,7 @@  class Series(dict):
             print(col.Color(col.RED, str))
 
     def MakeCcFile(self, process_tags, cover_fname, raise_on_error,
-                   add_maintainers):
+                   add_maintainers, limit=0):
         """Make a cc file for us to use for per-commit Cc automation
 
         Also stores in self._generated_cc to make ShowActions() faster.
@@ -238,6 +238,8 @@  class Series(dict):
                 print(col.Color(col.YELLOW, 'Skipping "%s"' % x))
             cc = set(cc) - set(settings.bounces)
             cc = [m.encode('utf-8') if type(m) != str else m for m in cc]
+            if limit > 0:
+                cc = cc[:limit]
             all_ccs += cc
             print(commit.patch, ', '.join(set(cc)), file=fd)
             self._generated_cc[commit.patch] = cc